views:

103

answers:

2

Okay, not sure what I'm doing here, other than it's not right. Trying to overload the '==' method of a class, and it's just... not working. At least, I get a false back from my main, and the cout in the implementation of '==' doesnt output.

These are my three files:

// TestClass.h

#ifndef TESTCLASS_H
#define TESTCLASS_H

class TestClass {
public:
    TestClass(int contents);
    TestClass(const TestClass& orig);
    virtual ~TestClass();
    bool operator==(const TestClass& other);
private:
    int contents;
};

#endif  /* TESTCLASS_H */



// TestClass.cpp

#include <iostream>

#include "TestClass.h"

TestClass::TestClass(int contents) {
    this->contents = contents;
}

TestClass::TestClass(const TestClass& orig) {
    this->contents = orig.contents;
}

TestClass::~TestClass() {
}

bool TestClass::operator ==(const TestClass& other) {
    std::cout << "COMPARING" << std::endl;
    return (contents == other.contents);
}


// Main.cpp

#include <cstdlib>
#include <iostream>

#include "TestClass.h"

using namespace std;

/*
 * 
 */
int main(int argc, char** argv) {

    TestClass* tc = new TestClass(1);
    TestClass* tc1 = new TestClass(1);

    cout << (tc == tc1) << endl;

    return 0;
}

So the question is - what have I done wrong? I apologise for what is probably a very silly mistake somewhere, but I just can't spot it.

+11  A: 

tc == tc1 compares pointer values. It "should" be *tc == *tc1, but I don't get why you'd dynamically allocate in the first place.

Automatic (stack) allocation is highly preferred, only dynamically allocate when you need the object to be independent of scope. (And then keep track of it with automatically allocated smart pointers, which will delete the pointer when it's appropriate.)


Also, the operator should be const, because it doesn't modify this:

//                                      vvvvv
bool operator==(const TestClass& other) const;

Even better, though, is a free function:

bool operator==(const TestClass& lhs, const TestClass& rhs);

Which would possibly be a friend. (Free-functions are always preferred, plus this allows 5 == tc to work.)

GMan
Thanks for the answer. I really should have spotted the pointer problem, sorry. However, your answer did also teach me other things, and leave me with a few questions - mostly about this 'dynamic' versus 'automatic' allocation. To be frank, I have no idea what you're talking about - google for 'C++ automatic allocation' gives a first result that claims dynamic and automatic allocation are the same thing (http://www.cplusplus.com/forum/articles/416/). So, sorry, but what are you actually talking about in regards to that?
Stephen
@Stephen, what GMan is referring to that instead of saying `TestClass* tc = new TestClass(1);` you can also write `TestClass tc(1);` and it will construct an object with the same value. This is the preferred way in C++ over new/delete. You also missed out on the delete for tc and tc1, so these objects will never be destroyed. C++ doesn't have a garbage collector like C# or Java does, you have to take out the garbage yourself.
Timo Geusch
@Stephen: That link makes no sense (that person obviously fails to understand the difference between C++ the language and implementations). I wish there were a super-delete button on the internet. In any case, dynamic, automatic, and static are *storage durations*. An automatic is the default, and objects with automatic duration will be destroys automatically (hence the name) at the end of their scope. Dynamic allocation is independent from scope, and you obtain a pointer to a dynamically allocated object via `new` (and friends.) If you never manually delete a dynamically allocated object...
GMan
@Stephen I recommend: stop. Read a book about C++. You will not get this stuff right by trial and error.
Steve Jessop
...its lifetime will never end, resulting in a leak. (You can end the lifetime of a dynamically allocated object via `delete` (and friends.)) In C++, automatically allocated objects are (on a typical platform) quickly allocated because they are on the stack, and *much* safer. (Impossible to leak!) Contrarily, dynamic allocation tends to be slower, and is easy to mess up. Modern C++ uses automatic objects to "own" objects, and will automatically free them when it's time. (They pass ownership around, or share it.) In any case, you need to start from the beginning. Get a good book and read.
GMan
@GMan: "fails to understand the difference between C++ the language and implementations" - not to mention, even for whatever implementation they're talking about, fails to understand the difference between "the programs STACK" and the heap. O. M. G.
Steve Jessop
@Steve: lol, I know. Don't you wish we could just sneak in and obliterate the post? @Stephen: We have a list [here](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). It sounds like you were coming from another language and thought you'd just sort see what's different. That won't work (hint: everything is different), you *have* to forget you know anything and start from the basics.
GMan
"much safer. (Impossible to leak!)" - once you recognise that they cease to exist when the function (or block) exits, and hence for example can't be returned by pointer or by reference. Since `main` won't be returning them, not a problem in this case.
Steve Jessop
@GMan: I'd settle for a kind of inverse Google sitemap, so that if I nominate a page rubbish it stops indexing it. I have this sneaking suspicion that any such system would be ruined by the same people who write the rubbish in the first place...
Steve Jessop
@Steve, GMan: Thanks for your help. I've been working my way through Stroustrup's "The C++ Programming Language", but I think I've been rushing and getting ahead of myself here. A friend suggested I try Meyer's "Effective C++" instead; he said it was more learning-friendly. Thanks for all the help, and for warning me about my use of new - I just went and had a good read of the first few pages of the Classes chapter of Stroustrup's book, and voila - automatic allocation (not that he calls it as such, he just doesnt actually mention `new` at all in the first few pages). Anyway, thanks guys.
Stephen
cplusplus.com is a terrible, terrible website and they get almost everything wrong, almost all of the time. It is a blight on the programming world.
Steve M
+4  A: 

You are comparing pointers. Try that instead:

cout << (*tc == *tc1) << endl;

Two remarks:

  • You should free allocated memory with delete, or use a smart pointer
  • You should declare operator== const:

    bool operator==(const TestClass& other) const

Samuel_xL
Two more remarks: * `operator==` should be a non-member function, and * good luck implementing `operator==` with dynamic polymorphism: http://www.drdobbs.com/184405053
Steve Jessop
+1 for the non-member reminder
Chubsdad