Overloading = gives me an error

Code:

void cSTRING::operator=(const cSTRING& str)
{	
	if (pszString)
		delete[] pszString;
	
	nLen=str.Length();
	pszString=new char [nLen+1];
	myStringCopy(pszString, str.getString());
}

void main()
{
	cSTRING* pString;
	cSTRING* pPal;

	pString = new cSTRING("This is my string");
	pPal = new cSTRING("bacon, no Cab");

	pString=pPal;
	pString->Display();

	delete pString;
	delete pPal;
}

*I only included relvant code*
When I try to compile this I get these errors:
c:\documents and settings\joe\my documents\college.lvl.2\cst8132\lab5\cstring.cpp(1 59) : error C2662: 'Length' : cannot convert 'this' pointer from 'const class cSTRING' to 'class cSTRING &'
Conversion loses qualifiers
c:\documents and settings\joe\my documents\college.lvl.2\cst8132\lab5\cstring.cpp(1 61) : error C2662: 'getString' : cannot convert 'this' pointer from 'const class cSTRING' to 'class cSTRING &'
Conversion loses qualifiers

Whats the problem with this?

Thanks

The assignment operator should return a const reference to an object, not void. And main() should return an int.

Originally posted by joshdick
The assignment operator should return a const reference to an object, not void. And main() should return an int.

While the latter is correct, and the former is considered proper (though it should return a non-const reference), they're not actually the problem here.

The problem is that the Length() member function isn't defined to be a const member-function, yet you are using it on the const reference to a cSTRING in the operator= function.

Length, which most-likely just returns the length of the string, is a function which won't change the state of the object, and so it has to be noted as such in order for it to work with a const object. In C++, you can call no member functions of an object when the object is const unless you explicitly lable the member functions as being const. This makes it so that when the this pointer is passed to the function, it is passed as a pointer to a const rather than a pointer to a non-const value. If you have a const object and you call a non-const function, that would mean that you'd have to convert from a pointer-to-const type to just a pointer-to-type, which is not safe (and illegal without casting in C++, which is why you get the error, and even if you cast it the behavior is usually undefined).

To label your Length member function as being const, put const after the function declaration and function header but before the function body.

So, change length to

Code:

std::size_t cSTRING::Length() const
{
  return nLen;
}

(and change it appropriately in the declaration in the class definition as well).

That should fix your problem. Try to be consistent in your use of const. If you never use it, you won't come into compile problems, but your code becomes less clear and potentially less safe. If you use it all the time, then your code is more safe and more clear. Either way you can get your code to run and compile. However, if you use const in some places (IE in your operator=) but not in other places (like your Length function), then you start running into problems like the one you just came across. Try to get into the habit of thinking about const-correctness every single time you make a function or variable. It makes everything much better in the end and saves headaches in the future.

Thanks for the help!!

I have a new problem though...When I debug this it doesn't go into my overloaded operator function. It copys the address of pPal to pString.

Code:

void main()
{
	cSTRING* pString;
	cSTRING* pPal;

	pString = new cSTRING("This is my string");
	pPal = new cSTRING("bacon, no Cab");

	pString=pPal;
	pString->Display();

	delete pString;
	delete pPal;
}

Thanks

Originally posted by KonArtis
Thanks for the help!!

I have a new problem though...When I debug this it doesn't go into my overloaded operator function. It copys the address of pPal to pString.

Thanks

That's because you're using pointers and copying the address. The assignment operator is called when you copy objects, not pointers. Don't dynamically allocate your objects here -- there is no reason to.

Code:

int main()
{
  cSTRING String("This is my string"), Pal("bacon, no Cab");

  String = Pal;
  pString.Display();
}

If, for some reason you wanted to use dynamic memory allocation like you were doing in your original example (which I wouldn't recommend), you can copy the object by doing

*pString = *pPal;

Again, though, instead of that, just work with the objects non-dynamically.


EDIT:

Also noting that in your example you have a memory leak and you deleted the same object twice. Both of which are very bad (latter generally worse than the former).