Add optional writeAll separator argument.

Add optional writeAll separator argument.

BUG= http://dartbug.com/8694

Committed: https://code.google.com/p/dart/source/detail?r=20943

[sra1, 2013-03-22 21:19:46]

Perhaps this could be coded more carefully (see Iterator.join, treat
separator==null like "" to allow forwarding; avoid tests in loop(s).)

A longer but more efficient version would be more palatable if there was only
one copy.  Do mixins work well enough to put implementations of everything but
StringSink.write on StringSink and use it as a mixin?  StringSink is simple - it
has no fields or constructors or type parameters.

--------
If isFirst was reversed in sense (isContining?) and also an optional parameter,
it would permit concatenation of lists during output:

sb.writeAll(apples, ',');
sb.writeAll(oranges, ',', apples.length > 0);

Building a new list could use the StringBuffer size:
sb = new StringBuffer();
sb.writeAll(apples, ',', sb.length > 0);
sb.writeAll(oranges, ',', sb.length > 0);
sb.writeAll(pears, ',', sb.length > 0);

--------
I notice also that Iterable.join is stringifying the argument to
StringBuffer.write, which seems redundant.

https://codereview.chromium.org/13008021/diff/1/sdk/lib/core/string_buffer.dart
File sdk/lib/core/string_buffer.dart (right):

https://codereview.chromium.org/13008021/diff/1/sdk/lib/core/string_buffer.da...
sdk/lib/core/string_buffer.dart:38: if (separator != "") write(separator);
Can we avoid this repeated test in the loop?
Or at least fold it with isFirst:

if (separator == "") separator = null;
String nextSeparator = null;
for (var obj in objects) {
  if (nextSeparator != null) write(nextSeparator);
  nextSeparator = separator;
  write(object);
}

[floitsch, 2013-03-26 18:30:23]

We are going to discuss the extra argument ("isContinuing") but I would prefer
not to add it.

I would also like to change the optional argument of join to be equal to "".

https://codereview.chromium.org/13008021/diff/1/sdk/lib/core/string_buffer.dart
File sdk/lib/core/string_buffer.dart (right):

https://codereview.chromium.org/13008021/diff/1/sdk/lib/core/string_buffer.da...
sdk/lib/core/string_buffer.dart:38: if (separator != "") write(separator);
On 2013/03/22 21:19:46, sra1 wrote:
> Can we avoid this repeated test in the loop?
> Or at least fold it with isFirst:
> 
> if (separator == "") separator = null;
> String nextSeparator = null;
> for (var obj in objects) {
>   if (nextSeparator != null) write(nextSeparator);
>   nextSeparator = separator;
>   write(object);
> }
> 

Done.

[Lasse Reichstein Nielsen, 2013-04-03 10:35:52]

lgtm

The good thing about separator is that it handles the annoying boilerplate for
outputting n-1 separators for n elements. If it's n separators too, a loop is
just as good.
That is: I don't like isContinuing. It buys nothing over:
  for (var o in something) {
    buffer..write(", ")..write(o);
  }

https://codereview.chromium.org/13008021/diff/1/sdk/lib/core/string_buffer.dart
File sdk/lib/core/string_buffer.dart (right):

https://codereview.chromium.org/13008021/diff/1/sdk/lib/core/string_buffer.da...
sdk/lib/core/string_buffer.dart:38: if (separator != "") write(separator);
If speed is of the essence, I'd prefer:
 void writeAll(Iterable objects, [String separator = ""]) {
   if (separator.isEmpty) {
     for (Object object in objects) {
       write(object);
     }
   } else {
     Iterator iterator = objects.iterator();
     if (!iterator.moveNext()) return;
     write(iterator.current);
     while (iterator.moveNext()) {
       write(separator);
       write(iterator.current);
     }
   }
 }

Or, at least initialize nextSeparator to "" and drop the test completely.

[floitsch, 2013-04-04 22:36:35]

https://codereview.chromium.org/13008021/diff/1/sdk/lib/core/string_buffer.dart
File sdk/lib/core/string_buffer.dart (right):

https://codereview.chromium.org/13008021/diff/1/sdk/lib/core/string_buffer.da...
sdk/lib/core/string_buffer.dart:38: if (separator != "") write(separator);
On 2013/04/03 10:35:52, Lasse Reichstein Nielsen wrote:
> If speed is of the essence, I'd prefer:
>  void writeAll(Iterable objects, [String separator = ""]) {
>    if (separator.isEmpty) {
>      for (Object object in objects) {
>        write(object);
>      }
>    } else {
>      Iterator iterator = objects.iterator();
>      if (!iterator.moveNext()) return;
>      write(iterator.current);
>      while (iterator.moveNext()) {
>        write(separator);
>        write(iterator.current);
>      }
>    }
>  }
> 
> Or, at least initialize nextSeparator to "" and drop the test completely.

I didn't use Stephen's suggestion but had a code that is pretty similar to
yours. Assuming as "done", but feel free to verify.

[floitsch, 2013-04-04 22:37:31]

Committed patchset #2 manually as r20943 (presubmit successful).