[webkit-reviews] review granted: [Bug 95502] Replace more instances of += with StringBuilder : [Attachment 161566] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Adam Barth <abarth at webkit.org>'s
request for review:
Bug 95502: Replace more instances of += with StringBuilder
https://bugs.webkit.org/show_bug.cgi?id=95502

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=161566&action=review


I suspect the InspectorFileSystemAgent.cpp change is technically incorrect
although unlikely to be a practical problem. Should be easy enough to fix.

> Source/WebCore/css/CSSTimingFunctionValue.cpp:50
> +    return "cubic-bezier(" +
> +	   String::number(m_x1) + ", " +
> +	   String::number(m_y1) + ", " +
> +	   String::number(m_x2) + ", " +
> +	   String::number(m_y2) + ")";

Our style is to put the + characters at the starts of the lines rather than the
ends.

> Source/WebCore/inspector/InspectorFileSystemAgent.cpp:584
> +    String result = decoder->decode(static_cast<char*>(buffer->data()),
buffer->byteLength()) + decoder->flush();

Please don’t make this change. I believe C++ does not guarantee order of
evaluation of arguments. So there is a chance that flush can be called here
before decode.

> Source/WebCore/inspector/InspectorStyleSheet.cpp:811
> +    styleSheetText.append(text);

Would be be better to construct styleSheetText with an initial value rather
than using append here?