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

Ulf Zibis Ulf.Zibis at CoSoCo.de
Wed Nov 12 10:36:29 UTC 2014


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




More information about the core-libs-dev mailing list