overloading the assigment operator

I recently compiled a string class (not my class, I must add, the one from SAMS 21 days)

I won't regurgitate the whole thing here, as my question is only specific to a certain section of the code, I'll try to give all that's needed.

Firstly, it has the private member variables

Code:

---

unsigned short len
char* itsString
String( unsigned short )

---

it has one overloaded assignment operator, which is as follows ( I've only written the definition here )

Code:

---

String& String::operator =(const String & rhs)
{
        if( this == &rhs )
                return *this;
        delete[] itsString;
        len = rhs.getLength();
        for( unsigned short i = 0; i<len; i++ )
                itsString[i] = rhs[i];
        itsString[len] = '\0';

        return *this;
}

---

So far so good. Then in the main function it does this.

Code:

---

char* temp = "Hello World";
s1 = temp //s1 has already been declared as a string

---

Now unless I'm mistaken, the argument for the assignment operator being used in this case is a C-string literal, and not a "String" as expected by our overloaded assignment operator. So here the compiler will make it's own assignment operator correct? Either way it compiles but when I run the program the debugger pops up and tells me there is a problem, and points to the destructor, which looks like this

Code:

---

String::~String()
{
        delete [] itsString;
        len = 0;
}

---

For convenience, here's the message which pops up, not that it's terribly interesting.

http://www.stonehambey.com/debugString.jpg

My initial thought was that the compiler generated assignment operator for C style strings didn't assign the private member variables properly and that is why there was a problem when the destructor on it is called when the program closes.

I tried writing my own additional assignment operator, which looked something like this

Code:

---

String& String::operator =(const char * rhs)
{
        if( itsString == rhs )
                return *this;
        delete[] itsString;

        len = strlen( rhs ) + 1;
        for( unsigned short i = 0; i<len; i++ )
                itsString[i] = rhs[i];
        itsString[len] = '\0';

        return *this;
}

---

But I'm still getting problems when debugging the program (of course it could be I haven't written the second assignment operator correctly ;) )

I'm pretty sure it's got something to do with that part of the main program, since when I comment it out it works fine, but I'm just not sure what.

Any advice or help would be greatly appreciated :)

Regards,

Stonehambey

Quote:

---

Now unless I'm mistaken, the argument for the assignment operator being used in this case is a C-string literal, and not a "String" as expected by our overloaded assignment operator. So here the compiler will make it's own assignment operator correct?

---

No. If one is not provided and yet it is used, the compiler will generate the copy assignment operator. However, the copy assignment operator has been provided, as you have shown its definition. What could happen is that the constructor that takes a char* (or related type) is invoked to construct a temporary, and this temporary is then copied via the copy assignment operator.

At a glance, the copy assignment operator as given in the book is flawed as it does not allocate memory, not to mention that it is not exception safe.

By providing an operator= that takes a const char *, your assignment operator is called, but it also fails to allocate memory and is not exception safe.

Your problem is just that you don't allocate any space for the new string in the assignment operator.
And no the compiler doesn't automatically generate an assignment operator that takes as const char * for you.
This only works because your string class propaply has a constructor that takes a const char *.
This constructor is used to create a temporary string object and then the assignment operator is called.
Kurt

I allocated the memory and it seemed to fix the problem.

It did, however, strike me as odd that such a widely known C++ tome would have such an error. So I re-read their assignment operator definition. Sure enough, embedded between two lines of code, was the memory allocation which I had, in my infinite wisdom, managed to completely overlook.

Exception handling is yet to be covered in the book, so I'm guessing they left it out for that reason.

I won't deny that I feel a bit of a fool now, but I'll still take on board what you told me about how assignment operators work, and make sure to double check the bloody book next time!

As always, thanks for replies :)

Quote:

---

Exception handling is yet to be covered in the book, so I'm guessing they left it out for that reason.

---

In this case making the copy assignment operator exception safe involves either implementing it in terms of the copy constructor, destructor and a swap function, or introducing a temporary pointer, and only assigning to len and using delete[] on itsString after initialising this temporary pointer with the return value of new[]. This way, even if new[] throws an exception, the state of the object remains unchanged.

I shot ahead and read the first couple of pages on handling errors and exceptions. I now see that when allocating memory it is wise to check for exceptions.

I think I understood the ways you described to check for exceptions in this case, the second more than the first, perhaps. However I won't attempt to write some bodged attempt at handling exceptions at the moment, since it's currently beyond my abilities, and probably wouldn't be a good idea.

I look forward to learning more about them now, shame it's the second to last chapter in the book. But I guess no point trying to run before you can walk eh? :)

Don't forget that string literals should always be const, as well:
http://cpwiki.sourceforge.net/Common...kes_and_errors

Quote:

---

Originally Posted by Elysia

Don't forget that string literals should always be const, as well:
http://cpwiki.sourceforge.net/Common...kes_and_errors

---

Nice link *bookmarked*

Thanks :)