<Swing Dev> 6303622: Generics: JComboBox: get/setSelectedItem(Object)

Florian Brunner fbrunnerlist at gmx.ch
Sun Jun 27 00:39:33 UTC 2010


Hi Pavel,

sorry for my long absence. After recovering from a fracture of the wrist earlier this year and organizing my removal to London, I hope I can find now more time again for this project.

To our discussion:
The usage of generified get/setSelectedItem for editable combo boxes should 
also be possible without a custom editor, if you don't mix types, eg. don't 
want to allow and render null values in a custom way.

Here is a further analysis of the situation:


---------------------------
1. Without generics (current state):

ComboBoxModel integerModel = ...;
JComboBox cb = new JComboBox(integerModel);
cb.setEditable(true);
Integer selectedInteger = (Integer) cb.getSelectedItem(); // might throw a 
ClassCastException
Object selectedObject = cb.getSelectedItem(); 
if (selectedObject instanceof Integer){ // explicit test needed
...
} else if (selectedObject instanceof String){
  try {
      Integer i = Integer.valueOf(newValue); // needed since "valueOf" gets 
not called in all cases in the default ComboBoxEditors
  } catch (NumberFormatException e) {
      ...
  }
}
---------------------------

2. Generified model only:

ComboBoxModel<Integer> integerModel = ...;
JComboBox<Integer> cb = new JComboBox<Integer>(integerModel);
cb.setEditable(true);
Integer selectedInteger = (Integer) cb.getSelectedItem(); // might throw a 
ClassCastException
Object selectedObject = cb.getSelectedItem(); 
if (selectedObject instanceof Integer){ // explicit test needed
...
} else if (selectedObject instanceof String){
  try {
      Integer i = Integer.valueOf(newValue); // needed since "valueOf" gets 
not called in all cases in the default ComboBoxEditors
  } catch (NumberFormatException e) {
      ...
  }
}

---------------------------

3. "Fully" generified version:

ComboBoxModel<Integer> integerModel = ...;
JComboBox<Integer> cb = new JComboBox<Integer>(integerModel);
cb.setEditable(true);
Integer selectedInteger = cb.getSelectedItem();  // might throw an unexpected 
ClassCastException; see remark below

---------------------------
As you see, with the current implementation of the "default" ComboBoxEditors 
provided by the JDK, assuming/ casting to a type when calling getSelectedItem can always 
result in a ClassCastException. The difference is that with 1. and 2. the cast 
is explicit and the developer can expect a ClassCastException here, while with 
example 3. the developer probably doesn't expect it.

How can this be, that this unexpected ClassCastException gets thrown, when we use generics?

If ComboBoxEditor is generified as well as get/setSelectedItem, and no raw 
version or other unchecked code is used anywhere, then:
- the compiler ensures that there will be no ClassCastException
- but since the ComboBoxEditor (unlike a ListCellRenderer) is not only a 
consumer but also a producer, it's not possible to provide a default 
implementation!

The only way to create instances of any type without some helper classes (such 
as eg. factories or interfaces) is to use reflection. The problem is that reflection can sidestep generics. And that's exactly the case with the default implementations of ComboBoxEditor in the JDK.

But as you can see with your example (see IntegerComboTestDefaultEditor.java for a slightly modified version), there is also another problem with the current implementations of ComboBoxEditor in the JDK: editing a JComboBox with a "default" ComboBoxEditor provided by the JDK, results in apparently non-deterministic output:

- type "foo" -> String
- replace with "1" + Enter -> String
- select 2 from the drop-down box -> Integer
- replace with "1" + Enter -> Integer

-> for the same input (replace with "1") you get once a String and once an 
Integer! That means even a correct input can be returned as String and has to be converted 
manually! 


We could fix this, however, eg. like this:
in the BasicComboBoxEditor replace:
----------------------------
    public Object getItem() {
        Object newValue = editor.getText();

        if (oldValue != null && !(oldValue instanceof String))  {
            // The original value is not a string. Should return the value in 
it's
            // original type.
            if (newValue.equals(oldValue.toString()))  {
                return oldValue;
            } else {
                // Must take the value from the editor and get the value and 
cast it to the new type.
                Class<?> cls = oldValue.getClass();
                try {
                    Method method = cls.getMethod("valueOf", new Class[]
{String.class});
                    newValue = method.invoke(oldValue, new Object[] { 
editor.getText()});
                } catch (Exception ex) {
                    // Fail silently and return the newValue (a String object)
                }
            }
        }
        return newValue;
    }
----------------------------

with:

----------------------------
  public Object getItem() {
        Object newValue = editor.getText();

        if (oldValue != null && !(oldValue instanceof String))  {
            // The original value is not a string. Should return the value in it's
            // original type.
            if (newValue.equals(oldValue.toString()))  {
                return oldValue;
            } else {
                // Must take the value from the editor and get the value and cast it to the new type.
                Class<?> cls = oldValue.getClass();
                try {
                    Method method = cls.getMethod("valueOf", new Class[]{String.class});
                    newValue = method.invoke(oldValue, new Object[] { editor.getText()});
                    return newValue;
                } catch (Exception ex) {
                    // Fail silently and return null
                }
            }
        }
        if (oldValue instanceof String) {
            return newValue;
        } else {
            return null;
        }
    }
----------------------------

This implementation might still not be perfect, but it has 2 advantages:
- it provides more consistent output (see examples IntegerComboTestFixedDefaultEditor.java and StringComboTestFixedDefaultEditor.java)
- it allows to generify the get/setSelectedItem methods without risking unexpected ClassCastExceptions

So I still vote for generifying get/setSelectedItem and ComboBoxEditor, too.

Regards,
Florian



Am Dienstag, 15. Dezember 2009 schrieb Pavel Porvatov:
> Hi Florian,
>
> > Hi Pavel,
> >
> > I start here a new thread for the "get/setSelectedItem(Object) methods of
> > JComboBox and ComboBoxModel" discussion.
> >
> > After further analysis of the code and your sample application I think we
> > can and should generify the get/setSelectedItem(Object) methods of
> > JComboBox and ComboBoxModel.
> >
> > Yes, the Javadoc says that JComboBox/ ComboBoxModel supports selected
> > values not managed by the underlying list model. But this does not
> > prohibit to optionally limit the type by using generics und thus to
> > increase type safety.
> >
> > If you need to allow other types from editor than the ones in the list
> > model, you still can use: JComboBox<Object> (or JComboBox, but this is
> > not recommended)
> >
> > So there should be no backward compatibility problem.
> >
> > When using a JComboBox, usually you are interested in the selected value
> > and since you want to do something with it you expect it to have some
> > specific type. So if we generify the get/setSelectedItem(Object), you can
> > profit from that in most cases.
> >
> > Even in cases where you have an initial text in an editable combo box you
> > can profit from that, if you use a "null" value as the selected value,
> > which according to the API is used for "no selection", and a custom
> > editor for rendering that null value. (see attachement; I used your
> > sample application as a base; delete the text to set the selected value
> > to null again).
>
> I agree that generification of the get/setSelectedItem(Object) methods
> will be useful. But than we will have another generification
> disadvantage. I tried to summarize benefits of two solutions below.
>
> *Generified get/setSelectedItem:*
> a. Simplified usage read-only comboboxes or non read-only comboboxes
> with special editor
>
> b. Disadvantage: if you use generified editable combobox *without*
> editor then ClassCastException will be thrown in runtime
>
> *Not generified get/setSelectedItem:*
> a. A possibility to generify the javax.swing.JComboBox#dataModel as
> ComboBoxModel<? extends E>. It give us more flexible usage of ComboBox:
>
> ComboBoxModel<Integer> cbModel = ....;
> JComboBox<Number> cb = new JComboBox<Number>(cbModel);
>
> Note that it's the main benefit that forced us to suggest not generified
> methods
>
> b. To use not read-only combobox with generified model
>
>
> So I believe that not generified get/setSelectedItem methods give more
> benefits and less disadvantages.
> What do you think about that?
>
> Regards, Pavel




-------------- next part --------------
A non-text attachment was scrubbed...
Name: FixedBasicComboBoxEditor.java
Type: text/x-java
Size: 5792 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20100627/7edb4af7/FixedBasicComboBoxEditor.java>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: StringComboTestFixedDefaultEditor.java
Type: text/x-java
Size: 2224 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20100627/7edb4af7/StringComboTestFixedDefaultEditor.java>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: IntegerComboTestFixedDefaultEditor.java
Type: text/x-java
Size: 2224 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20100627/7edb4af7/IntegerComboTestFixedDefaultEditor.java>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: IntegerComboTestDefaultEditor.java
Type: text/x-java
Size: 2249 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20100627/7edb4af7/IntegerComboTestDefaultEditor.java>


More information about the swing-dev mailing list