c++ char* reinitialising in operator=

Question

I would like to ask some question. I am taking OOP C++ course in my faculty. We had some exercise connected with reinitializing dynamicly allocated char array. The problem is as follows:

 class state
{
    private:
    char *szstate; // a name of object


    public:
    state &operator = (const state&);
};

state & state::operator = (const state &cop)
{
    if (this != &cop)
    {
        szstate = new char [strlen(cop.szstate)+strlen("==.")+1];
        strcpy(this->szstate,"==.");
        strcat(this->szstate, cop.szstate);

        return *this;
    }
    else return *this;

}

In shortcut, creating an object "a" with the name "DEFAULT" and after b = a, should give me object b with name "==.DEFAULT". This will need more memory, so I used operator new for safety. The thing is that teacher said there should be also

delete []szstate 

before I reinitialize the "name" of object - I tried this, but segmentation fault appeared. Anyway, I found some writing in Stephen Prata's book, where he wrote that char* shall remain untouched; char* shall be treated as unchangeable constant, so that will be some kind of explanation for the error. So, am I doing something wrong or the teacher have no right? Thanks for the answers :)

Answer 1

What's the initial value of szstate?

You should read up about the rule of three. Basically, if you need to hand write any of the three functions copy constructor, assignment and destructor, you probably have to write all of them. And have some non-trivial constructors as well. In your case:

• You clearly need constructors, to ensure that szstate is initialized to a defined value, always. (Depending on the logic, a null pointer may be OK.)

• You also need a destructor, to ensure that any dynamically allocated memory pointed to by szstate is correctly freed.

• In the assignment operator, you must ensure that you never leave the object in an invalid state if there is an exception.

The classical way of doing this would be:

class State
{
    char* myState;
public:
    State()
        : myState( new char[1] )
    {
        myState[0] = '\0';
    }

    State( State const& other )
        : myState( new char[ strlen( other.myState ) + 1 ] )
    {
        strcpy( myState, other.myState );
    }

    ~State()
    {
        delete[] myState;
    }

    State& operator=( State const& other )
    {
        char const* tmp = new char[ strlen( other.myState) + 1 ];
        strcpy( tmp, other.myState );
        delete myState;
        myState = tmp;
        return *this;
    }
};

This implements pure deep copy semantics, but it should give you an indication of what you need to do.

Also: there's no need to check for self assignment in the assignment operator. The need for such a check is often a sign that you're doing something wrong, which won't be safe if there are exceptions.

Answer 2

You should add constructor to state class and initialize in it szstate to nullptr (or 0), then in your operator= before new char call

delete[] szstate; // edit, no need to check szstate if its null (standard allows it)

this way you will not get segmentation fault

char* shall remain untouched; char* shall be treated as unchangeable constant, so that will be some kind of explanation for the error.

Maybe you mean string literals (const char[])? You should not delete those, but you have dynamic array which should be deleted once its no longer needed, or you you are about to loose pointer to it by assigning newly allocated array to it.