Monday, June 11, 2007

Cause and Effect

I have a tree-view (think like the tree of files in an Explorer or a Finder window). Since all of my rows are based on objects with integer IDs, I have something like this:

map mOpen;

bool Is_Open(int id)
{
map::iterator i = mOpen.find(id);
return (i = mOpen.end() ? true : i->second;
}


Note that if we don't have an ID in the map, we pretend its value is true. This "defaults" the hierarchy to open whenever we come upon a new ID - similarly "clearing" the map opens everybody.

So then I write this clever toggle routine:

void Toggle_Open(int id)
{
mOpen[id] = !Is_Open(id);
}


And it doesn't work. Why not? Well, it turns out that the l-value mOpen[id] is evaluated before the right-side expression. Thus we have already called mOpen[id] before we ever call Is_Open.

Here's the subtle bug: calling the array operator on a map creates a default entry (by the default ctor of the value type) if there is no entry for the key. So mOpen[id] on an id that isn't present in the map inserts a default 0 into the map.

Now Is_Open was modified to return "true" for unknown IDs, but this code is making the ID known and false.

The result was: the first time the user clicks on an open triangle, it stays open. This is because it is being set to closed by map::operator[] and then opened.

The correct code is

void Toggle_Open(int id)
{
bool old = Is_Open(id);
mOpen[id] = !old;
}


This forces the current open state to be evaluated before the array operator can make a default entry.

To quote from chapter 5 of the C++ spec:

"Except where noted, the order of evaluation of operands of individual operators and subexpressions of individual expressions and the order in which the side effects take place, is unspecified."


I knew the specification was good for something. :-)

No comments:

Post a Comment