Identity – The Often Overlooked Test Case
Today I had a very interesting bug that has been sitting in the server's code base for ages. In one of the support libraries there's a standard C++ string class modeled loosely after the Java String class. I inherited this code and never gave it much thought - until today.
The original code had an operator=() method implemented like this:
const jString & jString::operator( const jString & anOther ) {
empty();
append(anOther);
return *this;
}
Where the empty() method and the append(const jString &) methods worked perfectly. The problem came into play when I had something like this in the code:
jString one("hello");
one = one;Now this does not make a lot of sense, and in truth, the actual code was a lot more convoluted than this - but the point is that it's setting itself equal to itself. In any case, it shouldn't have done what it did. What it did was to clear out the value and leave the string empty. Why? Because the empty() method cleared out the value of anOther so that the append() call had nothing to copy from. This was a pain in the rear because I believed that the operator=() was smarter than this. It wasn't.
The fix is simple:
const jString & jString::operator( const jString & anOther ) {
if (this != & anOther) {
empty();
append(anOther);
}
return *this;
}
and so long as we check to make sure we aren't operating on ourselves, we're good to go. I know that there are probably a good number of cases like this in the code and I'm going to have to check each of the operator=() methods in the libraries, but at least I know what to fix, and the fix is easy.
I know that I'm going to be paying a lot more attention to the overlooked case of 'self' in the code I write from now on.