[OpenJDK 2D-Dev] [11] RFR: [JDK-8194489] Incorrect size computation at BandedSampleModel.createDataBuffer

[Jayathirth D V]

Hi Prahalad,

 

Change looks good to me.

 

Thanks,

Jay

 

From: Brian Burkhalter 
Sent: Wednesday, January 10, 2018 6:52 AM
To: Prahalad Kumar Narayanan
Cc: 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [11] RFR: [JDK-8194489] Incorrect size computation at BandedSampleModel.createDataBuffer

 

Hi Prahalad,

 

On Jan 9, 2018, at 3:42 AM, Prahalad Kumar Narayanan <HYPERLINK "mailto:prahalad.kumar.narayanan at oracle.com"prahalad.kumar.narayanan at oracle.com> wrote:





BandedSampleModel
L193-196 and 203-207: In general I think it's better for method-internal comments to use the "//" form instead of the "/*.*/" form.

  My views:
     . I find interspersed use of // and /* */ for multi-line comments in our client libs code.
     . For example: The D3DBlitLoops uses "//" while many parts of Image IO use /* ... */ for multi-line comments.
     . While implementing the fix, I try to maintain uniformity established by the file getting modified.

 

I think that is a good approach.





     . In general, I refer to this guideline- http://cr.openjdk.java.net/~alundblad/styleguide/index-v6.html

 

For the record I tend to prefer the "//" for intra-method comments as I have found that not infrequently in debugging one needs to comment out a section and this is much easier to do if the comments in question do not use the "/*.*/" style. I think you can leave this one as-is however.





BandedSampleModelSizeTest
Instead of defining the utility methods compareSamples(int[],int[]) and initSamples(int[],int) 
you might consider simply using j.u.Arrays.equals(int[],int[]) and j.u.Arrays.fill(int[],int), respectively.  

  My views:
     . Thank you for the suggestion. It helped.
     . I 've modified the test code to use Arrays.fill and Arrays.equals methods.

The modified code is now available for review under
Link: http://cr.openjdk.java.net/~pnarayanan/8194489/webrev.01/

Kindly review at your convenience and share your thoughts.

 

I think this looks OK.

 

Thanks,

 

Brian
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20180110/4cfb3aea/attachment.html>