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

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

Well, sure, I agree with the memory fragmentation in principle. But, let's face it, this just isn't going to make any noticeable difference. There are dozens (if not more) allocations elsewhere for each request.

What I find worrying here is are all the other changes, such as this one:

- throw parser.errorText() + "\n\n" + parser.helpText();
+ throw QString(parser.errorText() + "\n\n" + parser.helpText());

Adding the -DQT_USE_QSTRINGBUILDER caused previously working code to fail because, suddenly, what is thrown is a string builder instead of a string. I can't say I like this magic all that much. The same single expression (a + b) can return two different types, depending on whether the symbol is defined or not.

A less intrusive change would be to not do all the casts where the exceptions are thrown and instead add a catch handler for string builder in main? But, even then, this leaves a bad taste in my mouth. Changing the type of an expression via a preprocessor symbol is dangerously close to preprocessor abuse, IMO.

« Back to merge proposal