Hi Krishna,
This seems good to me except one thing. You are checking getRowCount()
== 0 but there is a chance of test extending VariableHeightLayoutCache
and overriding getRowCount to return -ve also as it is an int. In that
case, I guess this function will not return -1 which spec mandates [If
there are no rows, -1 is returned] so I guess we should check for <=0.
Also, there is no need of calling getRowCount() twice as it will not
change between 929, 936.
Regards
Prasanta
On 12/7/2017 4:41 PM, Krishna Addepalli wrote:
Hi Sergey,
Per our conversation, I have done the following changes:
1.Found that the .class size increases by 1kb when streams are used,
so reverted the changes related to it.
2.Moved the “++nextIndex” into the conditional, so that there is no
logical change.
Here is the updated webrev:
http://cr.openjdk.java.net/~kaddepalli/8190281/webrev01/
<http://cr.openjdk.java.net/%7Ekaddepalli/8190281/webrev01/>
Thanks,
Krishna
*From:* Krishna Addepalli
*Sent:* Wednesday, December 6, 2017 2:43 PM
*To:* swing-dev@openjdk.java.net
*Subject:* [10][JDK-8190281] Code cleanup in
src\java.desktop\share\classes\javax\swing\tree\VariableHeightLayoutCache.java
Hi All,
Please review the fix for bug:
Bug: JDK-8190281 https://bugs.openjdk.java.net/browse/JDK-8190281
JDK 10 Webrev:
http://cr.openjdk.java.net/~kaddepalli/8190281/webrev00/
<http://cr.openjdk.java.net/%7Ekaddepalli/8190281/webrev00/>
This bug was created while root causing JDK-8187936, and the following
refactoring points have been addressed:
1. Line 927: Uninitialized variables, checking for trivial reject case
multiple times.
2. Line 999: Traditional code written to find maximum size of
components, which can be done without any local variables and explicit
looping by replacing with streams.
3. Line 1365: Code repetition for differenct conditions, which can be
ored together to reduce the repetition.
4. Line 1482: A large code block gets repeated only because of
different values need to be passed in one line. This can be moved to a
variable initialization, and the repeating code blocks can be reduced
to one.
5. Line 1505: Variable initialization can be simplified by combining
different conditions.
6. Line 1540: An explicit loop to apply a function over a collection,
can be achieved in one line by a forEach construct. – This is
producing some visual artifacts, so ignored.
7. Line 1747: Combine all the trivial reject cases into one condition,
and also, a potential bug which increments the "nextIndex" value
beyond the length of the containing elements. The increment should
happen only if the trivial reject case fails.
Thanks,
Krishna
