Problems with overloaded '+' and '='

I have to do a homework that requires operator overloading. It's very simple , but i think i forgot alot about operator overloading.

I need to make a premitive string-like class that only has overloaded + and = signs.

Here's the header file :

Code:

#ifndef STRTYPE_H
#define STRTYPE_H


class StrType
{
public :

	StrType(char * = "\0");

	StrType &operator=(const StrType &);
	StrType operator+(const StrType &);

	void print(void);

	~StrType();

private :

	char *ptr;
};

#endif

and the .cpp file :

Code:

#include <iostream>
using std::cout;
using std::endl;

#include <cstring>

#include "strtype.h"


StrType::StrType(char *str)
{
	ptr = new char[strlen(str) + 1];

	strcpy(ptr, str);
}

StrType &StrType::operator=(const StrType &right)
{
	if( &right != this)
	{
	delete [] ptr; // error always occur here

	ptr = new char[strlen(right.ptr) + 1];

	strcpy(ptr, right.ptr);
	}

	return *this;
}

StrType StrType::operator+(const StrType &right)
{
	char *temp = new char[strlen(right.ptr) + strlen(ptr) + 1];

	strcpy(temp, ptr);
	strcat(temp, right.ptr);

	delete [] ptr;

	ptr = new char[strlen(temp) + 1];

	strcpy(ptr, temp);

	delete [] temp;

	return *this;
}


void StrType::print()
{
	cout << ptr;
}

StrType::~StrType()
{
	delete [] ptr;
}

Am i using the proper return types? My C++ book says that = should return a const reference of an object , but that doesn't make sense to me. Also , my operator= function always fail when trying to delete 'ptr' and show an error "debug assertion failed" in VC++. (hilighted in red)

I used to return a reference to an object in operator+ till i noticed that ' a = b + c + d' would modify objects b , c and d , so i let it return a copy of the object instead.


I just need help in those two operator's logic and return value.

Originally Posted by Brain Cell

Am i using the proper return types? My C++ book says that = should return a const reference of an object , but that doesn't make sense to me.

Your book is right. If you return a non-const reference, then someone could muck directly with your object when you don't want them to.

For an example on how to implement a string class, I suggest looking over apstring: http://www.stanford.edu/~brendano/misc/ap-c++/

Why are you messing with your object in the +operator? +operator should be a const operation.

Code:

StrType StrType::operator+(const StrType &right)
{
	char *temp = new char[strlen(right.ptr) + strlen(ptr) + 1];

	strcpy(temp, ptr);
	strcat(temp, right.ptr);

	delete [] ptr;  //why are you changing your object?

	ptr = new char[strlen(temp) + 1];

	strcpy(ptr, temp);

	delete [] temp;

	return *this;
}

You need to be returning a temporary object instead. If you want the compiler to check for problems like that, declare your function as const.

Code:

//in header
	StrType operator+(const StrType &) const;
//in implementation
	StrType StrType::operator+(const StrType &right) const

Additionally, implement a copy constructor. I promise that the one being generated by the compiler is not the one you want.

I would implement operator = in a different way:

Code:

const StrType &StrType:: operator=(const StrType &right)
{	
	if (strlen(right.ptr) > strlen(ptr)) 
    {                               
    char* newData = new char[strlen(right.ptr)+1];
    delete[] ptr;
    ptr = newData;
  }
  memcpy(ptr, right.ptr, strlen(right.ptr)+1);
  return *this;

}

Try this...

Originally Posted by joshdick

Your book is right. If you return a non-const reference, then someone could muck directly with your object when you don't want them to.

For an example on how to implement a string class, I suggest looking over apstring: http://www.stanford.edu/~brendano/misc/ap-c++/

Well it depends....
Here is what Mr. Eckel said:

All the assignment operators modify the lvalue. To allow the result of the assignment to be used in chained expressions, like a=b=c, it’s expected that you will return a reference to that same lvalue that was just modified. But should this reference be a const or nonconst? Although you read a=b=c from left to right, the compiler parses it from right to left, so you’re not forced to return a nonconst to support assignment chaining. However, people do sometimes expect to be able to perform an operation on the thing that was just assigned to, such as (a=b).func( ); to call func( ) on a after assigning b to it. Thus, the return value for all of the assignment operators should be a nonconst reference to the lvalue.

- Micko

It was Bruce Eckel.... not me I've just quoted second opinion. That practice is used in his book, and because it's my best C++ book I stick to his advice. Also Bjarne Stroustrup didn't use returning const reference in his examples of overlaoding operator =...
So I stick to this variant...

Oh, I see you deleted your post joshdick, but never mind

@Brain Cell: Consider changing the way you implemented operator +.
I would chose to return a new object StrType because you're also modify operand and I consider it as a bad practice.

- Micko

i tried this now :

Code:

const StrType &StrType::operator+(const StrType &right)
{
	StrType temp(ptr);

	strcat(temp.ptr, right.ptr);

	return temp;
}

and it gives me an error for 'a + b + c' saying :

error C2678: binary '+' : no operator defined which takes a left-hand operand of type 'const class StrType' (or there is no acceptable conversion)

What the hell is going on? the operator is already defined

You should give up from that implementaion immediately!
You're trying to return reference to a local object - it's a disaster!

its a const reference as the book (and the source joshdick posted) says..

WHAT SHALL I DO THEN?? *sobs*

Well I would implement operator+ something like this:

Code:

const StrType StrType::operator+(const StrType &right)const
{
    size_t i, len1,len2;
    len1 = strlen(ptr);
    len2 = strlen(right.ptr);
	char *temp = new char[strlen(right.ptr) + strlen(ptr) + 1];

	for (i = 0; i < len1; i++)
        temp[i] = ptr[i];
    for (i = 0; i <= len2; i++)
        temp[i + len1] = right.ptr[i];   

    StrType t(temp);
    
    delete [] temp;
	
    return t;
}

Why are you trying to return reference when implement op +.
Is that your task?
I think returning reference to modified operand is not wise thing to do...
But regarding your question about comp error...
aside that you tried to return reference to a local object I think I know why this is complier err. (although I'm not 100% sure)
Compiler "interprets" a+b+c from right to left so after (b+c) is calculated const reference is returned and when trying to evaluate (a+ (b+c)) on the right side there was a const reference, but function wasn't declared const and if it's not declared const compiler thought this would change const object and produced error.

For example look at this code

Code:

class Int
{
      int x;
      public: void foo(){}
      Int(){x=12;}
};

int main(void)
{
	const Int i;
	i.foo();
	return 0;
}

This will produce error because function foo was not declared as const