Code review comment for lp://staging/~aacid/thumbnailer/fast_string_concatenation

Revision history for this message
Michi Henning (michihenning) wrote :

I'm feeling a bit ambivalent about this change now. The original change picked up by the tool was:

- string inpath = (artist_ + "_" + album_).toStdString() + "_" + suffix;
+ string inpath = QString(artist_ + "_" + album_).toStdString() + "_" + suffix;

Fair enough, it's a little bit more efficient this way. But, in practice, it doesn't make one iota of difference because the cost of the string catenation is infinitesimal compared to the overall cost of the operation. (We are about to do a client-server round trip.)

And now we have all the other changes where the cast to QString is needed. I'm wondering whether that really is worth it. We have more cryptic code in quite a few places now, in return for a performance improvement that is immeasurably small. Is this the right trade-off?

review: Needs Information

« Back to merge proposal