Re: RFR: 8050091: LinkedList has incorrect implementation comment

2015-06-30 Thread Paul Sandoz
On Jun 26, 2015, at 10:04 PM, Martin Buchholz marti...@google.com wrote: Hi Ivan, I'd like you to do a code review. https://bugs.openjdk.java.net/browse/JDK-8050091 http://cr.openjdk.java.net/~martin/webrevs/openjdk9/LinkedList-invariant/ I would prefer if there was some text with the

Re: RFR: 8050091: LinkedList has incorrect implementation comment

2015-06-30 Thread Martin Buchholz
(I've already committed this change...) Paul and Ivan, I see your point, but perhaps writing a commented out dataStructureInvariants method should be some kind of best practice, at least for classes where we're not willing to pay the bytecode cost of having the assertions be real code. This is

Re: RFR: 8050091: LinkedList has incorrect implementation comment

2015-06-29 Thread Ivan Gerasimov
On 27.06.2015 21:54, Martin Buchholz wrote: On Sat, Jun 27, 2015 at 5:46 AM, Ivan Gerasimov ivan.gerasi...@oracle.com mailto:ivan.gerasi...@oracle.com wrote: Hi Martin! Thank you for this cleanup! Removal of wrong comments looks fine. But your webrev contains commented

Re: RFR: 8050091: LinkedList has incorrect implementation comment

2015-06-29 Thread Ivan Gerasimov
Hi Martin! I agree that since the commented code only contains an assert statement, it's unlikely to be confused with a real method. It is helpful to have the invariants documented, so yes, it looks good. Sincerely yours, Ivan On 30.06.2015 0:56, Martin Buchholz wrote: On Mon, Jun 29,

Re: RFR: 8050091: LinkedList has incorrect implementation comment

2015-06-27 Thread Ivan Gerasimov
Hi Martin! Thank you for this cleanup! Removal of wrong comments looks fine. But your webrev contains commented checkInvariants() method. Is it a leftover from debugging or something? Sincerely yours, Ivan On 26.06.2015 23:04, Martin Buchholz wrote: Hi Ivan, I'd like you to do a code review.

Re: RFR: 8050091: LinkedList has incorrect implementation comment

2015-06-27 Thread Martin Buchholz
On Sat, Jun 27, 2015 at 5:46 AM, Ivan Gerasimov ivan.gerasi...@oracle.com wrote: Hi Martin! Thank you for this cleanup! Removal of wrong comments looks fine. But your webrev contains commented checkInvariants() method. Is it a leftover from debugging or something? It could be used for

RFR: 8050091: LinkedList has incorrect implementation comment

2015-06-26 Thread Martin Buchholz
Hi Ivan, I'd like you to do a code review. https://bugs.openjdk.java.net/browse/JDK-8050091 http://cr.openjdk.java.net/~martin/webrevs/openjdk9/LinkedList-invariant/