Re: RFR: 8274261: Use enhanced-for instead of plain 'for' in jdk.jcmd
On Fri, 24 Sep 2021 07:30:02 GMT, Andrey Turbanov wrote: > There are few places in code where manual `for` loop is used with Iterator to > iterate over Collection or Array. > Instead of manual `for` cycles it's preferred to use enhanced-for cycle > instead: it's less verbose, makes code easier to read and it's less > error-prone. > It doesn't have any performance impact: javac compiler generates similar code > when compiling enhanced-for cycle. > > One strange thing I also noticed is static field > `sun.tools.jstat.Parser#reservedWords`, which filled in `Parser` constructor. > Reworked to initialize it once. Marked as reviewed by cjplummer (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5673
Re: RFR: 8274261: Use enhanced-for instead of plain 'for' in jdk.jcmd
On Fri, 24 Sep 2021 09:01:28 GMT, Andrey Turbanov
wrote:
>> src/jdk.jcmd/share/classes/sun/tools/jstat/OptionFormat.java line 81:
>>
>>> 79:
>>> 80: for (Iterator i = children.iterator();
>>> i.hasNext(); /* empty */) {
>>> 81: OptionFormat o = i.next();
>>
>> Why did not you simplify the lines 80-81 the same way as in line 85?
>
> It can't be simplified: it calls `Iterator.hasNext()` inside cycle body.
> `i.hasNext()` at line 82
Okay. Thank you for explanation.
-
PR: https://git.openjdk.java.net/jdk/pull/5673
Re: RFR: 8274261: Use enhanced-for instead of plain 'for' in jdk.jcmd
On Fri, 24 Sep 2021 08:45:54 GMT, Serguei Spitsyn wrote:
>> There are few places in code where manual `for` loop is used with Iterator
>> to iterate over Collection or Array.
>> Instead of manual `for` cycles it's preferred to use enhanced-for cycle
>> instead: it's less verbose, makes code easier to read and it's less
>> error-prone.
>> It doesn't have any performance impact: javac compiler generates similar
>> code when compiling enhanced-for cycle.
>>
>> One strange thing I also noticed is static field
>> `sun.tools.jstat.Parser#reservedWords`, which filled in `Parser`
>> constructor. Reworked to initialize it once.
>
> src/jdk.jcmd/share/classes/sun/tools/jstat/OptionFormat.java line 81:
>
>> 79:
>> 80: for (Iterator i = children.iterator();
>> i.hasNext(); /* empty */) {
>> 81: OptionFormat o = i.next();
>
> Why did not you simplify the lines 80-81 the same way as in line 85?
It can't be simplified: it calls `Iterator.hasNext()` inside cycle body.
`i.hasNext()` at line 82
-
PR: https://git.openjdk.java.net/jdk/pull/5673
Re: RFR: 8274261: Use enhanced-for instead of plain 'for' in jdk.jcmd
On Fri, 24 Sep 2021 07:30:02 GMT, Andrey Turbanov
wrote:
> There are few places in code where manual `for` loop is used with Iterator to
> iterate over Collection or Array.
> Instead of manual `for` cycles it's preferred to use enhanced-for cycle
> instead: it's less verbose, makes code easier to read and it's less
> error-prone.
> It doesn't have any performance impact: javac compiler generates similar code
> when compiling enhanced-for cycle.
>
> One strange thing I also noticed is static field
> `sun.tools.jstat.Parser#reservedWords`, which filled in `Parser` constructor.
> Reworked to initialize it once.
Looks good.
One place was not simplified in the OptionFormat.java (see my inlined comment).
Thanks,
Serguei
src/jdk.jcmd/share/classes/sun/tools/jstat/OptionFormat.java line 81:
> 79:
> 80: for (Iterator i = children.iterator(); i.hasNext();
> /* empty */) {
> 81: OptionFormat o = i.next();
Why did not you simplify the lines 80-81 the same way as in line 85?
-
Marked as reviewed by sspitsyn (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/5673
RFR: 8274261: Use enhanced-for instead of plain 'for' in jdk.jcmd
There are few places in code where manual `for` loop is used with Iterator to iterate over Collection or Array. Instead of manual `for` cycles it's preferred to use enhanced-for cycle instead: it's less verbose, makes code easier to read and it's less error-prone. It doesn't have any performance impact: javac compiler generates similar code when compiling enhanced-for cycle. One strange thing I also noticed is static field `sun.tools.jstat.Parser#reservedWords`, which filled in `Parser` constructor. Reworked to initialize it once. - Commit messages: - [PATCH] Use enhanced-for instead of plain 'for' in jdk.jcmd Changes: https://git.openjdk.java.net/jdk/pull/5673/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5673&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274261 Stats: 44 lines in 6 files changed: 0 ins; 14 del; 30 mod Patch: https://git.openjdk.java.net/jdk/pull/5673.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5673/head:pull/5673 PR: https://git.openjdk.java.net/jdk/pull/5673
