Sign in to follow this  

Overloading ==?

This topic is 4269 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I'm not sure if I'm doing this right:
#include <string>

class CStudent
{
public:

	CStudent( std::string sName, int nClass, int nHouse )
        {
        	this->m_sName = sName;
	        this->m_nClass = nClass;
	        this->m_nHouse = nHouse;
        }

	std::string	m_sName;
	
	int m_nClass;
	int	m_nHouse;

	CStudent* pOpponent1;
	CStudent* pOpponent2;
	CStudent* pOpponent3;
	CStudent* pOpponent4;

	inline bool operator == (CStudent sOpponent)
	{
		if( this->m_sName == sOpponent.m_sName )
		{
			return true;
		}
		else
		{
			return false;
		}
	}
};

But trying this doesn't work:
        CStudent* one;
	CStudent* two;

	one = new CStudent( "Name", 1, 2 );
	two = new CStudent( "Name", 1, 2 );

	if( one == two )
	{
		MessageBox( NULL, "Equal", "", MB_OK);
	}

What am I doing wrong when overloading ==? It is a bool right?

Share this post


Link to post
Share on other sites
Quote:
Original post by Smit
I know about the leak, I only wrote that as a quick test.

So how should I pass them to ==?


Since you are using pointers, try derefencing them first. Like this:

if (*one == *two) ...


EDIT: Bah, too slow.

Share this post


Link to post
Share on other sites
As Roboguy said you are comparing pointers. To compare what they point at you need to dereference them:

if (*one == *two)


You could also just create the objects on the stack.

CStundent one("Name", 1, 2);
CStundent two("Name", 1, 2);
if (one == two)

A secondary problem is that the the overloaded == is not defined properly, it's not causing a problem here, but this is how it should be declared:
inline bool operator == (const CStudent& sOpponent) const
{
}

Note that the 'inline' is redundant, you could remove it.

Share this post


Link to post
Share on other sites
Try this.


#include <string>

class CStudent
{
public:

CStudent(std::string sName, int nClass, int nHouse)
{
sName = sName;
m_nClass = nClass;
m_nHouse = nHouse;
}

std::string m_sName;
int m_nClass;
int m_nHouse;

CStudent *pOpponent1;
CStudent *pOpponent2;
CStudent *pOpponent3;
CStudent *pOpponent4;

inline bool operator==(const CStudent &sOpponent) const
{
return (m_sName == sOpponent.m_sName)

}

};




by using the expression itself as the return value it makes for more concise code. sOpponent is passed using a reference and const. Passing by reference (that is the & symbol) means that you actually need an object, but when you compare the two values (this is shown in the next bit of code) that a new object is not constructed and destructed. const shows that the object is non-modifiable in the context of the function. The second const shows that you dont modify the object refered to as *this .

Now about calling your equality operator, try this:


CStudent *one = new CStudent("Name", 1, 2);
CStudent *two = new CStudent("Name", 1, 2);

if(*one == *two) // this line is important
{
MessageBox(NULL, "Equal", 0, MB_OK);

}

delete one;
delete two;




This will work because you are comparing the objects not the pointers to the objects. new returns a pointer, not an object. That is you did wrong before, just correct the marked line. The other info I wrote is just better form.

EDIT: I'll remember not to write such a long response next time.

Share this post


Link to post
Share on other sites
Quote:
Original post by Shannon Barber
You could also just create the objects on the stack.

CStundent one("Name", 1, 2);
CStundent two("Name", 1, 2);


Quoted for emphasis. In C++, think before you 'new'. There are a lot of rules of thumb that go "Use X when you can; use pointers when you have to" - for good reason.

Share this post


Link to post
Share on other sites
Indeed, consider declaring on the stack whenever possible. Not only is it less prone to memory leak errors, it is also faster to allocate variables on the stack compared to storing variables on the heap.

Share this post


Link to post
Share on other sites

This topic is 4269 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this