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
/*
* Copyright (c) 1997, 2008, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation. Oracle designates this
* particular file as subject to the "Classpath" exception as provided
* by Oracle in the LICENSE file that accompanied this code.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
package combotest2;
import javax.swing.ComboBoxEditor;
import javax.swing.JTextField;
import javax.swing.border.Border;
import java.awt.Component;
import java.awt.event.*;
import java.lang.reflect.Method;
/**
* The default editor for editable combo boxes. The editor is implemented as a JTextField.
*
* @author Arnaud Weber
* @author Mark Davidson
*/
public class FixedBasicComboBoxEditor implements ComboBoxEditor,FocusListener {
protected JTextField editor;
private Object oldValue;
public FixedBasicComboBoxEditor() {
editor = createEditorComponent();
}
public Component getEditorComponent() {
return editor;
}
/**
* Creates the internal editor component. Override this to provide
* a custom implementation.
*
* @return a new editor component
* @since 1.6
*/
protected JTextField createEditorComponent() {
JTextField editor = new BorderlessTextField("",9);
editor.setBorder(null);
return editor;
}
/**
* Sets the item that should be edited.
*
* @param anObject the displayed value of the editor
*/
public void setItem(Object anObject) {
String text;
if ( anObject != null ) {
text = anObject.toString();
oldValue = anObject;
} else {
text = "";
}
// workaround for 4530952
if (! text.equals(editor.getText())) {
editor.setText(text);
}
}
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;
}
}
public void selectAll() {
editor.selectAll();
editor.requestFocus();
}
// This used to do something but now it doesn't. It couldn't be
// removed because it would be an API change to do so.
public void focusGained(FocusEvent e) {}
// This used to do something but now it doesn't. It couldn't be
// removed because it would be an API change to do so.
public void focusLost(FocusEvent e) {}
public void addActionListener(ActionListener l) {
editor.addActionListener(l);
}
public void removeActionListener(ActionListener l) {
editor.removeActionListener(l);
}
static class BorderlessTextField extends JTextField {
public BorderlessTextField(String value,int n) {
super(value,n);
}
// workaround for 4530952
public void setText(String s) {
if (getText().equals(s)) {
return;
}
super.setText(s);
}
public void setBorder(Border b) {
if (!(b instanceof UIResource)) {
super.setBorder(b);
}
}
}
/**
* A subclass of FixedBasicComboBoxEditor that implements UIResource.
* FixedBasicComboBoxEditor doesn't implement UIResource
* directly so that applications can safely override the
* cellRenderer property with BasicListCellRenderer subclasses.
* <p>
* <strong>Warning:</strong>
* Serialized objects of this class will not be compatible with
* future Swing releases. The current serialization support is
* appropriate for short term storage or RMI between applications running
* the same version of Swing. As of 1.4, support for long term storage
* of all JavaBeans<sup><font size="-2">TM</font></sup>
* has been added to the <code>java.beans</code> package.
* Please see {...@link java.beans.XMLEncoder}.
*/
public static class UIResource extends FixedBasicComboBoxEditor
implements javax.swing.plaf.UIResource {
}
}
package combotest2;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.awt.event.FocusEvent;
import java.awt.event.FocusListener;
import javax.swing.JButton;
import javax.swing.JComboBox;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.SwingUtilities;
public class StringComboTestFixedDefaultEditor {
private static void createGui() {
final JFrame frame = new JFrame();
frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
final JComboBox cb = new JComboBox(new String[]{"foo", "bar", "42"});
cb.setEditable(true);
cb.setEditor(new FixedBasicComboBoxEditor());
cb.setSelectedItem(null);
JPanel panel = new JPanel();
panel.add(cb);
cb.addActionListener(new ActionListener() {
public void actionPerformed(ActionEvent e) {
System.out.print("Action: ");
printCurrentSelectedItem(cb);
}
});
cb.getEditor().getEditorComponent().addFocusListener(new FocusListener() {
public void focusGained(FocusEvent e) {
System.out.print("Focus : ");
printCurrentSelectedItem(cb);
}
public void focusLost(FocusEvent e) {
System.out.print("Focus : ");
printCurrentSelectedItem(cb);
}
});
JButton button = new JButton("Focus change");
panel.add(button);
frame.add(panel);
frame.setSize(400, 200);
frame.setLocationRelativeTo(null);
frame.setVisible(true);
}
private static void printCurrentSelectedItem(JComboBox cb) {
if (cb.getSelectedItem() == null) {
System.out.println("cb.getSelectedItem() = null");
} else {
System.out.println("cb.getSelectedItem() = " + cb.getSelectedItem().
getClass() + ", " + cb.getSelectedItem());
}
}
public static void main(String[] args) throws Exception {
SwingUtilities.invokeAndWait(new Runnable() {
public void run() {
StringComboTestFixedDefaultEditor.createGui();
}
});
}
}
package combotest2;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.awt.event.FocusEvent;
import java.awt.event.FocusListener;
import javax.swing.JButton;
import javax.swing.JComboBox;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.SwingUtilities;
public class IntegerComboTestFixedDefaultEditor {
private static void createGui() {
final JFrame frame = new JFrame();
frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
final JComboBox cb = new JComboBox(new Integer[]{1, 2, 3});
cb.setEditable(true);
cb.setEditor(new FixedBasicComboBoxEditor());
cb.setSelectedItem(null);
JPanel panel = new JPanel();
panel.add(cb);
cb.addActionListener(new ActionListener() {
public void actionPerformed(ActionEvent e) {
System.out.print("Action: ");
printCurrentSelectedItem(cb);
}
});
cb.getEditor().getEditorComponent().addFocusListener(new FocusListener() {
public void focusGained(FocusEvent e) {
System.out.print("Focus : ");
printCurrentSelectedItem(cb);
}
public void focusLost(FocusEvent e) {
System.out.print("Focus : ");
printCurrentSelectedItem(cb);
}
});
JButton button = new JButton("Focus change");
panel.add(button);
frame.add(panel);
frame.setSize(400, 200);
frame.setLocationRelativeTo(null);
frame.setVisible(true);
}
private static void printCurrentSelectedItem(JComboBox cb) {
if (cb.getSelectedItem() == null) {
System.out.println("cb.getSelectedItem() = null");
} else {
System.out.println("cb.getSelectedItem() = " + cb.getSelectedItem().
getClass() + ", " + cb.getSelectedItem());
}
}
public static void main(String[] args) throws Exception {
SwingUtilities.invokeAndWait(new Runnable() {
public void run() {
IntegerComboTestFixedDefaultEditor.createGui();
}
});
}
}
package combotest2;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.awt.event.FocusEvent;
import java.awt.event.FocusListener;
import javax.swing.JButton;
import javax.swing.JComboBox;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.SwingUtilities;
public class IntegerComboTestDefaultEditor {
private static void createGui() {
final JFrame frame = new JFrame();
frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
final JComboBox cb = new JComboBox(new Integer[]{1, 2, 3});
cb.setEditable(true);
System.out.println("Editor: " + cb.getEditor().getClass().getName());
cb.setSelectedItem(null);//"Initial value");
JPanel panel = new JPanel();
panel.add(cb);
cb.addActionListener(new ActionListener() {
public void actionPerformed(ActionEvent e) {
System.out.print("Action: ");
printCurrentSelectedItem(cb);
}
});
cb.getEditor().getEditorComponent().addFocusListener(new FocusListener() {
public void focusGained(FocusEvent e) {
System.out.print("Focus : ");
printCurrentSelectedItem(cb);
}
public void focusLost(FocusEvent e) {
System.out.print("Focus : ");
printCurrentSelectedItem(cb);
}
});
JButton button = new JButton("Focus change");
panel.add(button);
frame.add(panel);
frame.setSize(400, 200);
frame.setLocationRelativeTo(null);
frame.setVisible(true);
}
private static void printCurrentSelectedItem(JComboBox cb) {
if (cb.getSelectedItem() == null) {
System.out.println("cb.getSelectedItem() = null");
} else {
System.out.println("cb.getSelectedItem() = " + cb.getSelectedItem().
getClass() + ", " + cb.getSelectedItem());
}
}
public static void main(String[] args) throws Exception {
SwingUtilities.invokeAndWait(new Runnable() {
public void run() {
IntegerComboTestDefaultEditor.createGui();
}
});
}
}
