help with classes

Hi,
i am trying to use overloading operators with C-strings, and i keep crashing when i enter the +operator

Here is my header file:

Code:

#ifndef __cDATE_H
#define __cDATE_H

#include <iostream.h>

class cSTRING 
{
	private:
		int _nCmpValue, _num1, _num2;
		char *pszName, *pszName1;
	public:
		cSTRING();
		
		cSTRING(char *pname, int returnVal);
		cSTRING(char *pszStr1);
		
		cSTRING(cSTRING &rhs);			
				
		~cSTRING();

		char *getName()
		{
			return pszName;
		}

		int getVal()
		{
			return _nCmpValue;
		}
		

		
		void two(char*,int);
		void print()
	    {
		    cout << _num1 << endl << _num2 << endl;
	    }


		cSTRING& operator=(const cSTRING& cStr);
		cSTRING operator+(const cSTRING& cStr);
		cSTRING& operator+=(const cSTRING& cStr);
		int operator==(const cSTRING& cStr);
		cSTRING& operator!=(const cSTRING& cStr);
};

#endif

my CPP File:

Code:

#include "cDATE.h"
#include <iostream.h>
#include <string.h>

cSTRING::cSTRING(char *name, int returnVal)
{
	_nCmpValue = returnVal;
	
	pszName = new char[strlen(name) + 1];
     //Allocates an character array object
    strcpy(pszName, name);

	cout << "cSTRING Default Constructor Called" << endl;
}

cSTRING::cSTRING(char *name)
{
	pszName = 0;
	pszName = new char[strlen(name) + 1];
	strcpy(pszName, name);
	cout << "Name copied" << endl;	
}

cSTRING cSTRING::operator+(const cSTRING& rhs)
{
	cout << "inside +" << endl;

	return cSTRING(strcat(pszName, rhs.pszName));      
}

cSTRING::cSTRING(cSTRING &rhs)
{
	_nCmpValue = rhs.getVal();
    pszName = new char[strlen(rhs.getName()) + 1];
    strcpy(pszName,rhs.pszName);
}

cSTRING::~cSTRING()
{ 
	if (pszName)	
		delete[] pszName;
	cout << "Destructor called" << endl; 
};



cSTRING::cSTRING()
{
}

my main file:

Code:

#include "cDATE.h"
#include <iostream.h>

int main(void)
{
	char szStr[] = "Hello";
	char szStr2[] = "SHello";

	int returnVal = 3;

	cSTRING setName(szStr);
	cSTRING setName1(szStr2);

	cout << "Now adding...." << endl;
	cSTRING oAdd = setName + setName1;
	cout << endl << endl << endl;
	oAdd.print();

	return (0);
}

basically i declare to strings that are dynamically allocated, and then try to concatanate them, but it doesnt work :(
i am guessing that i am not passing them right....or that i have not declared to seperate strings.

I suspect you need to allocate more space in this->pszName when you try to concatenate rhs.pszName onto this->pszName with the call to strcat() in the constructor in the return statement of the + operator. from what I can tell, this->pszName only has enough room for the string it was given at time of construction of its' construction, and never expanded to allow room for the addition.

You might want to consider implementing operator+ in terms of operator+=. So in operator+= you just strcat the string (after allocating the right amount of space) and in operator+ you create a temporary copy and call += with the passed in value. This probably won't improve efficiency at all, but if you ever decide to change your code you will only have to change one of the functions.

After doing what elad suggested, a good idea would be to return a reference the same object in your operator functions.

Code:

cSTRING& cSTRING::operator+(const cSTRING& rhs)
{
    cout << "inside +" << endl;

    //Allocate more space for pszName here

    strcat(pszName, rhs.pszName);

    return *this; // <--
}

It would improve efficiency because there are no constructor calls.

Originally Posted by The Dog

After doing what elad suggested, a good idea would be to return a reference the same object in your operator functions.

Code:

cSTRING& cSTRING::operator+(const cSTRING& rhs)
{
    cout << "inside +" << endl;

    //Allocate more space for pszName here

    strcat(pszName, rhs.pszName);

    return *this; // <--
}

It would improve efficiency because there are no constructor calls.

Wait... that is NOT a good idea.

It modifies the left hand side of the "+", which is not the natural behavior of operator+. Your code is better suited for an operator+=.

Oh, sorry 'bout that. I wasn't takin' note. Thanks for pointing it out.

If you want to remove the constructor call for efficiency reasons (which is probably not even necessary) you should do it on the client end. Assuming operator+= is implemented:

Code:

cSTRING setName(szStr);
cSTRING setName1(szStr2);

cSTRING oAdd = setName;
oAdd += setName1;

No temporary is created.

thanks for replies;

i tired to do what you guys suggessted but am getting an exception error: c000005
this error occurs when i declare:

Code:

cSTRING oAdd = setName;

here is my code for operator =

Code:

cSTRING& cSTRING::operator=(const cSTRING& other)
{
	cout << "inside =" << endl;
	cSTRING temp;
	if (this!=&other)
	{
		strcpy(pszName1, other.pszName1);
	}
	return *this;
}

This code:

Code:

cSTRING oAdd = setName;

does not call operator=, it calls the copy constructor:

Code:

cSTRING(cSTRING &rhs) { ... }

I don't see anything wrong with your copy constructor in the first post, so maybe you should show your current code and exactly where it is getting that exception.

You could also use the debugger to help narrow down the problem.

[EDIT] - Oh, and I didn't even look at your operator=. You didn't allocate space for the new string. That code will only work if the current string in *this is longer than the one in rhs. Also, you aren't using the Temp cSTRING.