[KPhotoAlbum] An observation on the performance implications of constness

[Shawn Willden]

Here's something I noticed that is probably worth pointing out just so others 
can keep it in mind.  Maybe it's obvious to everyone but me, but it wasn't 
obvious to me, so I'll arrogantly assume that others might also learn 
something from it :-)

There's a trivial accessor method in Viewer::ViewerWidget:

DB::ImageInfoPtr Viewer::ViewerWidget::currentInfo()
{
    return DB::ImageDB::instance()->info(_list[ _current]);
}

This method is interesting because an artifact of Qt's QStringList 
implementation causes _list to be copied in many cases during the call, but 
it's not obvious that this might happen.  In this case it's not a big deal, 
because the list is not likely to be very large (unless you're in the habit 
of viewing a slideshow of your entire image database), but there may be other 
cases in which it matters more.

Lots of Qt data structures use implicit sharing with copy-on-write semantics.  
That's really nice because copying them is very fast, and yet you don't have 
to worry about unexpected modifications to shared data.  If one piece of code 
modifies such implicitly-shared data, it gets its own copy.  QStringList is 
implicitly shared.

It would seem that the currentInfo() method does not modify _list, so even if 
_list is shared (and it is), no copy should be made.  The problem, though, is 
that QStringList's operator[]() has no way of knowing whether or not the 
reference it returns will be modified, so it "detaches" the list just to be 
safe.  That means that if the list is shared, operator[]() will copy the list 
before returning a reference to the selected element, so that any 
modifications won't affect the other sharers.

There's a really easy way to prevent this:  use the const version of 
operator[]() instead of the non-const version.  One way to do that is:

DB::ImageInfoPtr Viewer::ViewerWidget::currentInfo()
{
    const QStringList& constList = _list;
    return DB::ImageDB::instance()->info(constList[ _current]);
}

but perhaps a better way is simply to declare currentInfo() as const, since it 
shouldn't really modify the ViewerWidget object state anyway.  It's perhaps 
surprising but true that:

DB::ImageInfoPtr Viewer::ViewerWidget::currentInfo() const
{
    return DB::ImageDB::instance()->info(_list[ _current]);
}

Is faster than the original version that doesn't have the "const" keyword on 
the end.  Like I said, it won't matter in this case unless _list is very 
large, but it's worth keeping in mind for other circumstances where it might 
have a more significant impact.

I'm a fan of "const correctness" just because of the beneficial impact it 
tends to have on code structure and clarity, but it can occasionally have 
performance implications as well.

	Shawn.