[webkit-reviews] review granted: [Bug 95508] Remove all-but-one use of WTF::String::operator+= from WebCore : [Attachment 161579] Patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has granted Adam Barth
<abarth at webkit.org>'s request for review:
Bug 95508: Remove all-but-one use of WTF::String::operator+= from WebCore
https://bugs.webkit.org/show_bug.cgi?id=95508

Attachment 161579: Patch
https://bugs.webkit.org/attachment.cgi?id=161579&action=review

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=161579&action=review


I am only half convinced we should have String::append() at all.

This is an improvement in any case.

> Source/WebCore/platform/chromium/support/WebHTTPLoadInfo.cpp:109
> -	   result.iterator->second += "\n" + String(value);
> +	   result.iterator->second.append("\n" + String(value));

One of those ugly case where the verbose way is better:
    result.iterator->second.append = result.iterator->second.append + '\n' +
String(value);

> Source/WebCore/platform/chromium/support/WebURLResponse.cpp:264
> -	   result.iterator->second += ", " + valueStr;
> +	   result.iterator->second.append(", " + valueStr);

The ugly case.

> Source/WebCore/xml/XMLHttpRequest.cpp:922
> -	   result.iterator->second += ", " + value;
> +	   result.iterator->second.append(", " + value);

Another case for
    result.iterator->second.append = result.iterator->second.append + ", " +
value);

> Source/WebCore/xml/XPathFunctions.cpp:-565
> -    // FIXME: Building a String a character at a time is quite slow.
>      for (unsigned i1 = 0; i1 < s1.length(); ++i1) {
>	   UChar ch = s1[i1];
>	   size_t i2 = s2.find(ch);
>	   
>	   if (i2 == notFound)
> -	       newString += String(&ch, 1);
> -	   else if (i2 < s3.length()) {
> -	       UChar c2 = s3[i2];
> -	       newString += String(&c2, 1);
> -	   }

:)