[webkit-reviews] review denied: [Bug 187963] String(View) should have a splitAllowingEmptyEntries function instead of a flag parameter : [Attachment 345744] Patch

[bugzilla-daemon at webkit.org]

Alex Christensen <achristensen at apple.com> has denied Ross Kirsling
<ross.kirsling at sony.com>'s request for review:
Bug 187963: String(View) should have a splitAllowingEmptyEntries function
instead of a flag parameter
https://bugs.webkit.org/show_bug.cgi?id=187963

Attachment 345744: Patch

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




--- Comment #6 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 345744
  --> https://bugs.webkit.org/attachment.cgi?id=345744
Patch

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

Cool!

> Source/WTF/wtf/text/WTFString.h:282
> +    WTF_EXPORT_PRIVATE void splitAllowingEmptyEntries(const String&
separator, Vector<String>& result) const;
> +    WTF_EXPORT_PRIVATE void splitAllowingEmptyEntries(UChar separator,
Vector<String>& result) const;

Let's get rid of these return-by-reference functions in favor of the
return-by-value functions.  All uses of them create a vector on the line before
calling them.

> Source/WTF/wtf/text/WTFString.h:285
> +    Vector<String> splitAllowingEmptyEntries(UChar separator) const;
> +    Vector<String> splitAllowingEmptyEntries(const String& separator) const;

Ideally even these would just return an iterator of some kind so we don't need
to allocate a Vector, because when we use this we really want to iterate it
quickly.  That might be better for a future patch, though.

> Tools/ChangeLog:9
> +	   Add tests for String::split and String::splitWithEmptyEntries.

String::splitAllowingEmptyEntries