[webkit-reviews] review granted: [Bug 194752] Continue reducing use of String::format, now focusing on hex: "%p", "%x", etc. : [Attachment 362293] Patch

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted Darin Adler <darin at apple.com>'s
request for review:
Bug 194752: Continue reducing use of String::format, now focusing on hex: "%p",
"%x", etc.
https://bugs.webkit.org/show_bug.cgi?id=194752

Attachment 362293: Patch

https://bugs.webkit.org/attachment.cgi?id=362293&action=review




--- Comment #14 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 362293
  --> https://bugs.webkit.org/attachment.cgi?id=362293
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=362293&action=review

> Source/WTF/wtf/text/StringConcatenateNumbers.h:119
> +	   : m_number(number)

No change necessary. Uniform initializer syntax (UIS)?

> Source/WebCore/html/HTMLMediaElement.h:1210
> +    static String toString(const
WebCore::HTMLMediaElement::AutoplayEventPlaybackState reason) { return
convertEnumerationToString(reason); }

Ok as-is. Const not necessary.

> Source/WebCore/html/HTMLMediaElement.h:1217
> +    static String string(WebCore::TextTrackCue* const& cue) { return
cue->debugString(); }

Ok as-is. const type without const pointer and the ref would be better. Find
yourself with more free time, it would be better to pass by const lvalue ref.

> Source/WebCore/platform/graphics/Color.cpp:393
> +    if (alpha() < 0xFF)
> +	   return makeString('#', hex(red(), 2), hex(green(), 2), hex(blue(),
2), hex(alpha(), 2));
> +    return makeString('#', hex(red(), 2), hex(green(), 2), hex(blue(), 2));

Nice.

> Source/WebCore/rendering/RenderFragmentedFlow.h:287
> +    static String string(const WeakPtr<WebCore::RenderFragmentContainer>
value) { return value.get() ? value->debugString() : String(); }

No change necessary. .get() is not necessary. Could use UIS for nullptr branch.