My apologies Sandro, Greg is correct, the compiler does not optimise
this case properly (checked with JDK1.6.0_14)

-- Noel.

Greg Brown wrote:
> I believe the compiler is capable of optimizing this:
>
> String s = "a" + "b" + "c";
>
> but not necessarily this:
>
> String s = "a";
> s += "b";
> s += "c";
>
> So FindBugs may not have been wrong to suggest the change. However,
> one case was a test app and the other was a tutorial (where clarity is
> much more important than performance), so please revert this change.
> Thanks.
>
> G
>
> On Nov 17, 2009, at 8:23 AM, Noel Grandin wrote:
>
>>
>> You're wasting your time and making the code less clear.
>>
>> See here:
>>   http://c2.com/cgi/wiki?StringBuffer
>>
>> This is why it is a bad idea to blindly follow the FindBugs report.
>>
>>
>> [email protected] wrote:
>>> Author: smartini
>>> Date: Tue Nov 17 13:19:52 2009
>>> New Revision: 881271
>>>
>>> URL: http://svn.apache.org/viewvc?rev=881271&view=rev
>>> Log:
>>> string concatenation improvement (as suggested by FindBugs)
>>>
>>> Modified:
>>>   
>>> incubator/pivot/trunk/tutorials/src/org/apache/pivot/tutorials/lists/ListViews.java
>>>
>>>
>>> Modified:
>>> incubator/pivot/trunk/tutorials/src/org/apache/pivot/tutorials/lists/ListViews.java
>>>
>>> URL:
>>> http://svn.apache.org/viewvc/incubator/pivot/trunk/tutorials/src/org/apache/pivot/tutorials/lists/ListViews.java?rev=881271&r1=881270&r2=881271&view=diff
>>>
>>> ==============================================================================
>>>
>>> ---
>>> incubator/pivot/trunk/tutorials/src/org/apache/pivot/tutorials/lists/ListViews.java
>>> (original)
>>> +++
>>> incubator/pivot/trunk/tutorials/src/org/apache/pivot/tutorials/lists/ListViews.java
>>> Tue Nov 17 13:19:52 2009
>>> @@ -51,7 +51,7 @@
>>>         }
>>>
>>>         private void updateSelection(ListView listView) {
>>> -            String selectionText = "";
>>> +            StringBuffer selectionText = new StringBuffer("");
>>>
>>>             Sequence<Span> selectedRanges =
>>> listView.getSelectedRanges();
>>>             for (int i = 0, n = selectedRanges.getLength(); i < n;
>>> i++) {
>>> @@ -61,15 +61,15 @@
>>>                     j <= selectedRange.end;
>>>                     j++) {
>>>                     if (selectionText.length() > 0) {
>>> -                        selectionText += ", ";
>>> +                        selectionText.append(", ");
>>>                     }
>>>
>>>                     String text =
>>> (String)listView.getListData().get(j);
>>> -                    selectionText += text;
>>> +                    selectionText.append(text);
>>>                 }
>>>             }
>>>
>>> -            selectionLabel.setText(selectionText);
>>> +            selectionLabel.setText(selectionText.toString());
>>>         }
>>>     };
>>>
>>>
>>>
>>>
>>
>

Reply via email to