Thanks for the review Sergey, but I have already updated the test appropriately. Also, please approve the CSR.
Krishna -----Original Message----- From: Sergey Bylokhov Sent: Wednesday, April 25, 2018 3:42 AM To: Krishna Addepalli <[email protected]>; Andrej Golovnin <[email protected]> Cc: [email protected] Subject: Re: <Swing Dev> [11][JDK-4842658] RFR: DefaultListModel and DefaultComboBoxModel should support addAll (Collection c) Looks fine. Please update the test as well before the push(it contains 'addAllElements, addAllElementsAt"). On 17/04/2018 21:00, Krishna Addepalli wrote: > Hi Sergey, > > Here is the new webrev with the api names changed to “addAll” in > DefaultComboBoxModel.java: > http://cr.openjdk.java.net/~kaddepalli/4842658/webrev05/ > > Thanks, > Krishna > > -----Original Message----- > From: Sergey Bylokhov > Sent: Wednesday, April 18, 2018 3:21 AM > To: Krishna Addepalli <[email protected]>; Andrej Golovnin > <[email protected]> > Cc: [email protected] > Subject: Re: <Swing Dev> [11][JDK-4842658] RFR: DefaultListModel and > DefaultComboBoxModel should support addAll (Collection c) > > On 12/04/2018 22:34, Krishna Addepalli wrote: >> Yes, I saw those methods in DefaultListModel, but it had add method, which >> is why I have added "addAll" method there. In the case of >> DefaultComboBoxModel, the add method is "addElement", which is why I chose >> to rename the plural methods to "addElements", and "addElementsAt". >> However, I'm ok to change the method names to addAll. > > Since these methods do similar thing, and both classes have the same parent > AbstractListModel, I suggest to use the same name either addElementsAt or > addAll. I think addAll is better, because it is aligned with the common names > from Collection framework. > >> >> Thanks, >> Krishna >> >> -----Original Message----- >> From: Sergey Bylokhov >> Sent: Friday, April 13, 2018 2:59 AM >> To: Krishna Addepalli <[email protected]>; Andrej Golovnin >> <[email protected]> >> Cc: [email protected] >> Subject: Re: <Swing Dev> [11][JDK-4842658] RFR: DefaultListModel and >> DefaultComboBoxModel should support addAll (Collection c) >> >> On 10/04/2018 05:43, Krishna Addepalli wrote: >>> - Why in one class you use addElements/addElementsAt names and in another >>> addAll? >>> In the DefaultComboBoxModel, the functions to add a single element are >>> named as addElement, addElementAt. >>> To keep with that convention, I named the apis appropriately. >> >> DefaultListModel also has getElementAt()/addElement()/removeElement() >> etc. I can guess this is because it is a wrapper of Vector which has >> the same methods. But in jdk1.2(when Collection framework was added) >> some additional methods were added to DefaultListModel like >> clear()/get()/set()/add() etc. >> I don't urge to change the existing methods in DefaultComboBoxModel but it >> would be good to use one common name pattern for models which is used in >> Collection framework. Any thoughts? >> >> >> -- >> Best regards, Sergey. >> > > -- Best regards, Sergey.
