JDK-8201289 : DefaultListModel and DefaultComboBoxModel should support addAll (Collection c)
  • Type: CSR
  • Component: client-libs
  • Sub-Component: javax.swing
  • Priority: P4
  • Status: Closed
  • Resolution: Approved
  • Fix Versions: 11
  • Submitted: 2018-04-09
  • Updated: 2018-05-01
  • Resolved: 2018-05-01
Related Reports
CSR :  
Description
Summary
-------

Add methods addAll, which adds a set of elements in a collection to the JList and JComboBox.

Problem
-------

There is currently no straight forward way of adding a Collection to a DefaultListModel and DefaultComboBoxModel. Shown below is the typical way clients add multiple elements to the model:

DefaultListModel<Integer> lm = new DefaultListModel<>();
List<Integer> lst = IntStream.range(0, 50).collect(ArrayList::new, ArrayList::add, ArrayList::addAll);

for (int i =0; i < lst.size(); i++) {
          lm.add(i);
}

The above method requires more code from the client side, and also results in unnecessary events being generated for each insertion.

Solution
-------

Solution is to add new api addAll, that takes a collection of items and add them all at once.  Here is the webrev containing the proposed changes: http://cr.openjdk.java.net/~kaddepalli/4842658/webrev05/

Specification
-------

Providing the public methods that are being affected here: 

1. src/java.desktop/share/classes/javax/swing/DefaultComboBoxModel.java

```
    /**
     * Adds all of the elements present in the collection.
     * 
     * @param c the collection which contains the elements to add
     * @throws NullPointerException if {@code c} is null
     */
    public void addAll(Collection<? extends E> c)

    /**
     * Adds all of the elements present in the collection, starting
     * from the specified index.
     * 
     * @param index index at which to insert the first element from the
     * specified collection
     * @param c the collection which contains the elements to add
     * @throws ArrayIndexOutOfBoundsException if {@code index} does not 
     * fall within the range of number of elements currently held
     * @throws NullPointerException if {@code c} is null
     */
    public void addAll(int index, Collection<? extends E> c)
```
   2. src/java.desktop/share/classes/javax/swing/DefaultListModel.java

```
    /**
     * Adds all of the elements present in the collection to the list.
     *
     * @param c the collection which contains the elements to add
     * @throws NullPointerException if {@code c} is null
     */
    public void addAll(Collection<? extends E> c)

    /**
     * Adds all of the elements present in the collection, starting
     * from the specified index.
     * 
     * @param index index at which to insert the first element from the 
     * specified collection
     * @param c the collection which contains the elements to add
     * @throws ArrayIndexOutOfBoundsException if {@code index} does not
     * fall within the range of number of elements currently held
     * @throws NullPointerException if {@code c} is null
     */
    public void addAll(int index, Collection<? extends E> c)
```
Comments
Moving to Approved based on the 05 version of the proposed change.
01-05-2018

Finalized it since Phil and Sergey have given a +1
25-04-2018

Api names changed to "addAll" in DefaultComboBoxModel.java as per Sergey's suggestion.
17-04-2018

Looks fine on the generics front. Please have one or more other swing engineers review the request and then Finalize it to go through the second phase of CSR review.
12-04-2018

Modified the tags to the new format, and also changed the doc, as suggested by Phil.
12-04-2018

Webrev, with apis modified to take any objects of class extending type E.
11-04-2018

Yes, I mean `foo<? extends E>` (that text is present in the raw form of my comment, but not rendered as such). I will not approve the addition of these new methods if do not take a generics wildcard. Only accepting `Collection<E>` is unnecessarily restrictive and does not follow the pattern used in the collections API, including the underlying addAll method on Vector.
10-04-2018

Hi Joe, Thanks for your comments. I presume you meant this: ```public void addAll(int index, Collection<? extends E> c)``` If that is so, I thought about it, but when I looked around, the constructors take a vector/array of objects of type E and not any object extending E. So, to keep things consistent, I have made the API, as it currently stands. If you think we should change the API, I guess we should make it in a separate change, since we may need to change other methods? I have changed the exception to throw ArrayIndexOutOfBoundsException. Added the null-input behavior in the doc. Thanks, Krishna
10-04-2018

Updated webrev, as per suggestions provided by Sergey.
10-04-2018

Before this request is finalized, please have one or more swing engineers review the request. Rather than public void addAll(int index, Collection<E> c) the generics should be public void addAll(int index, Collection<? extends E> c) As ultimately each element is being added, it can be of type E or a subtype. This is also the generics typing used on java.util.Collection.addAll and its overrides in Vector and elsewhere. Should an out-of-bounds index throw an IndexoOutOfBounds exception rather than an IllegalArgumentException? Is the null-input behavior specified at a package-level or somewhere else? As these methods can be overridden, some thought should be given to documenting which other methods they call. Moving to Provisional.
09-04-2018

Webrev, with changes as suggested by Andrej.
09-04-2018