Nit fixes in codegen_java_types.dart to clean up nits in generated code.

Nit fixes in codegen_java_types.dart to clean up nits in generated code.

R=paulberry@google.com, scheglov@google.com

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

[scheglov, 2014-08-14 06:50:22]

LGTM

https://codereview.chromium.org/473793002/diff/1/editor/tools/plugins/com.goo...
File
editor/tools/plugins/com.google.dart.server/src/com/google/dart/server/generated/types/SourceChange.java
(right):

https://codereview.chromium.org/473793002/diff/1/editor/tools/plugins/com.goo...
editor/tools/plugins/com.google.dart.server/src/com/google/dart/server/generated/types/SourceChange.java:120:
builder.append(selection.toString());
Actually, do we need to call toString() explicitly at all?
StringBuilder.append(Object) does it already.

[Paul Berry, 2014-08-14 12:58:02]

lgtm

https://codereview.chromium.org/473793002/diff/1/editor/tools/plugins/com.goo...
File
editor/tools/plugins/com.google.dart.server/src/com/google/dart/server/generated/types/SourceChange.java
(right):

https://codereview.chromium.org/473793002/diff/1/editor/tools/plugins/com.goo...
editor/tools/plugins/com.google.dart.server/src/com/google/dart/server/generated/types/SourceChange.java:114:
builder.append(message + ", ");
With a little extra effort we could probably change the output to:

  builder.append("[message=");
  builder.append(message);
  builder.append(", edits=");
  builder.append(StringUtils.join(edits, ", "));
  builder.append(", selection=");
  builder.append(selection);
  builder.append("]");

Which would have the advantage of avoiding the string reallocation that 'message
+ ", "' would have performed.

I'm splitting hairs at this point, though.  Your call.

https://codereview.chromium.org/473793002/diff/1/editor/tools/plugins/com.goo...
editor/tools/plugins/com.google.dart.server/src/com/google/dart/server/generated/types/SourceChange.java:120:
builder.append(selection.toString());
On 2014/08/14 06:50:22, scheglov wrote:
> Actually, do we need to call toString() explicitly at all?
> StringBuilder.append(Object) does it already.

If Jaime takes my suggestion above, that would be a good idea.

Otherwise, we'd have to be careful to keep the call to toString() in the case
where we're appending ' + ", "'.  Which means we could only do this optimization
on the last field of the object.  Not sure if it's worth doing at that point.

Again, I'm splitting hairs.  Your call, Jaime.

[scheglov, 2014-08-14 14:25:12]

https://codereview.chromium.org/473793002/diff/1/editor/tools/plugins/com.goo...
File
editor/tools/plugins/com.google.dart.server/src/com/google/dart/server/generated/types/SourceChange.java
(right):

https://codereview.chromium.org/473793002/diff/1/editor/tools/plugins/com.goo...
editor/tools/plugins/com.google.dart.server/src/com/google/dart/server/generated/types/SourceChange.java:120:
builder.append(selection.toString());
On 2014/08/14 12:58:02, Paul Berry wrote:
> On 2014/08/14 06:50:22, scheglov wrote:
> > Actually, do we need to call toString() explicitly at all?
> > StringBuilder.append(Object) does it already.
> 
> If Jaime takes my suggestion above, that would be a good idea.
> 
> Otherwise, we'd have to be careful to keep the call to toString() in the case
> where we're appending ' + ", "'.  Which means we could only do this
optimization
> on the last field of the object.  Not sure if it's worth doing at that point.
> 
> Again, I'm splitting hairs.  Your call, Jaime.

Actually it works even when we're appending '+ ", "'.

    if (true) {
      Object o = Lists.newArrayList(1, 2, 3);
      System.out.println(o + ", ");
      return;
    }

prints: '[1, 2, 3], '.

[jwren, 2014-08-14 18:42:20]

https://codereview.chromium.org/473793002/diff/1/editor/tools/plugins/com.goo...
File
editor/tools/plugins/com.google.dart.server/src/com/google/dart/server/generated/types/SourceChange.java
(right):

https://codereview.chromium.org/473793002/diff/1/editor/tools/plugins/com.goo...
editor/tools/plugins/com.google.dart.server/src/com/google/dart/server/generated/types/SourceChange.java:114:
builder.append(message + ", ");
On 2014/08/14 12:58:02, Paul Berry wrote:
> With a little extra effort we could probably change the output to:
> 
>   builder.append("[message=");
>   builder.append(message);
>   builder.append(", edits=");
>   builder.append(StringUtils.join(edits, ", "));
>   builder.append(", selection=");
>   builder.append(selection);
>   builder.append("]");
> 
> Which would have the advantage of avoiding the string reallocation that
'message
> + ", "' would have performed.
> 
> I'm splitting hairs at this point, though.  Your call.

Acknowledged.

https://codereview.chromium.org/473793002/diff/1/editor/tools/plugins/com.goo...
editor/tools/plugins/com.google.dart.server/src/com/google/dart/server/generated/types/SourceChange.java:120:
builder.append(selection.toString());
All "toString()"s removed.

[jwren, 2014-08-14 18:43:42]

Committed patchset #4 manually as 39262 (presubmit successful).