<Swing Dev> [10][JDK-8190281] Code cleanup in src\java.desktop\share\classes\javax\swing\tree\VariableHeightLayoutCache.java

[Prasanta Sadhukhan]

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 at 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
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20171209/45dc49ed/attachment.html>