Review request: JDK-8055723 Replace concat String to append in StringBuilder parameters

Otávio Gonçalves de Santana otaviojava at java.net
Thu Nov 13 00:57:44 UTC 2014


But this class is an Exception, doesn't make sense an exception get another
Exception.
IMHO: I prefer this way

On Wed, Nov 12, 2014 at 8:36 AM, Ulf Zibis <Ulf.Zibis at cosoco.de> wrote:

> Hi Otávio,
> I now think you could replace
>          if (!expected.isEmpty())
> with
>          assert !expected.isEmpty();
>
> If expected ever would be empty, the only thing which happens is, that a
> "'" is missing in a message which anyway doesn't make sense without
> arguments.
>
> -Ulf
>
>
>
> Am 12.11.2014 um 08:19 schrieb Otávio Gonçalves de Santana:
>
>> Thank you Ulf.
>> http://cr.openjdk.java.net/~weijun/8055723/webrev.01/ <
>> http://cr.openjdk.java.net/%7Eweijun/8055723/webrev.01/>
>>
>>
>> On Sat, Nov 8, 2014 at 3:46 PM, Ulf Zibis <Ulf.Zibis at cosoco.de <mailto:
>> Ulf.Zibis at cosoco.de>> wrote:
>>
>>     Hi Otávio,
>>     in sun/tools/jstat/SyntaxException.java I see a possible enhencement
>> (maybe applies to other
>>     places too):
>>
>>       65     public SyntaxException(int lineno, Set<String> expected,
>> Token found) {
>>       66         StringBuilder msg = new StringBuilder(A + B *
>> expected.size());
>>       67
>>       68         msg.append("Syntax error at line
>> ").append(lineno).append(": Expected one of \'");
>>       69
>>       71         for (String keyWord : expected) {
>>       72             msg.append(keyWord).append('|');
>>       73         }
>>       74         // if (!expected.isEmpty()) // only needed if expected
>> may be empty.
>>       75             msg.setLength(msg.length() - 1);
>>       76
>>       81         message = msg.append("\', Found
>> ").append(found.toMessage()).toString();
>>       83     }
>>
>>     ***** Additionally at many places you could similarly introduce the
>> foreach syntax.
>>
>>     -Ulf
>>
>>
>>     Am 02.11.2014 um 15:45 schrieb Otávio Gonçalves de Santana:
>>
>>         Could another reviewer look these codes, please.
>>         http://cr.openjdk.java.net/~weijun/8055723/webrev.00/
>>         <http://cr.openjdk.java.net/%7Eweijun/8055723/webrev.00/>
>>         <http://cr.openjdk.java.net/%7Eweijun/8055723/webrev.00/>
>>
>>         On Fri, Oct 24, 2014 at 3:25 AM, Otávio Gonçalves de Santana <
>> otaviojava at java.net
>>         <mailto:otaviojava at java.net> <mailto:otaviojava at java.net <mailto:
>> otaviojava at java.net>>> wrote:
>>
>>             Thank you Ulf.
>>             I removed the fix in toString method and in debug classes:
>>         http://cr.openjdk.java.net/~weijun/8055723/webrev.00/
>>         <http://cr.openjdk.java.net/%7Eweijun/8055723/webrev.00/>
>>             <http://cr.openjdk.java.net/%7Eweijun/8055723/webrev.00/>
>>
>>             On Mon, Oct 20, 2014 at 10:26 PM, Ulf Zibis <
>> Ulf.Zibis at cosoco.de
>>         <mailto:Ulf.Zibis at cosoco.de> <mailto:Ulf.Zibis at cosoco.de <mailto:
>> Ulf.Zibis at cosoco.de>>>
>>             wrote:
>>
>>
>>                 Am 21.10.2014 um 01:02 schrieb Otávio Gonçalves de
>> Santana:
>>
>>                     BUGURL: https://bugs.openjdk.java.net/
>> browse/JDK-8055723
>>
>>
>>                     WEBREV: http://cr.openjdk.java.net/~
>> weijun/8055723/client/webrev.02/
>>         <http://cr.openjdk.java.net/%7Eweijun/8055723/client/webrev.02/>
>>                     <http://cr.openjdk.java.net/%7Eweijun/8055723/client/
>> webrev.02/>
>>                     WEBREV: http://cr.openjdk.java.net/~
>> weijun/8055723/core/webrev.03/
>>         <http://cr.openjdk.java.net/%7Eweijun/8055723/core/webrev.03/>
>>                     <http://cr.openjdk.java.net/%
>> 7Eweijun/8055723/core/webrev.03/>
>>
>>
>>                 I did not look through all sources.
>>                 In Scanner.java I discovered:
>>                 1307 sb.append("[delimiters=").
>> append(delimPattern).append(']');
>>                 1308  sb.append("[position=").
>> append(position).append(']');
>>                 ...
>>                 Maybe better:
>>                 1307  sb.append("[delimiters=").append(delimPattern);
>>                 1308  sb.append("][position=").append(position);
>>                 ...
>>
>>                 -Ulf
>>
>>
>>
>>
>>             --     Otávio Gonçalves de Santana
>>
>>             blog: http://otaviosantana.blogspot.com.br/
>>             twitter: http://twitter.com/otaviojava
>>             site: _http://about.me/otaviojava_
>>         55 (11) 98255-3513 <tel:55%20%2811%29%2098255-3513>
>> <tel:55%20%2811%29%2098255-3513>
>>
>>
>>
>>
>>         --         Otávio Gonçalves de Santana
>>
>>         blog: http://otaviosantana.blogspot.com.br/
>>         twitter: http://twitter.com/otaviojava
>>         site: _http://about.me/otaviojava_
>>         55 (11) 98255-3513 <tel:55%20%2811%29%2098255-3513>
>> <tel:55%20%2811%29%2098255-3513>
>>
>>
>>
>>
>>
>> --
>> Otávio Gonçalves de Santana
>>
>> blog: http://otaviosantana.blogspot.com.br/
>> twitter: http://twitter.com/otaviojava
>> site: _http://about.me/otaviojava_
>> 55 (11) 98255-3513
>>
>
>


-- 
Otávio Gonçalves de Santana

blog:     http://otaviosantana.blogspot.com.br/
twitter: http://twitter.com/otaviojava
site:     *http://about.me/otaviojava <http://about.me/otaviojava>*
55 (11) 98255-3513



More information about the core-libs-dev mailing list