Reputation: 31730
I'm working on a project that lets you create products in a database. It includes a form with an editable combo box so you can select a pre-existing manufacturer for the new product, or type in a name of a new manufacturer you want to create along with the product.
The combo box is populated with a collection of Manufacturer objects (they implement toString() so they display something meaningful).
The logic for dealing with the output from the combobox is currently implemented thus:
Object mfr = mfctrCombo.getSelectedItem ();
Product newPrd = new Product ();
// If the mfr is a string then we need to create a new Manufacturer
if (mfr instanceof String) {
Manufacturer newMfr = new Manufacturer ();
newMfr.setName ((String) mfr);
// Logic for persisting the new Manufacturer goes here
newPrd.setManufacturer (newMfr);
} else if (mfr instanceof Manufacturer) {
newPrd.setManufacturer ((Manufacturer) mfr);
}
// Logic for persisting the new Product goes here
This does work, but the need to cast the mfr object seems unnecessary to me. I'm doing an instanceof check at the start of the if block, so I know for a fact what type the object is inside the block. Is it really necessary to do the cast inside the block after doing the check at the start of it as well? It seems to me that it shouldn't be required.
While I'm new to Java I'm fairly sure that what I'm doing with the combo box isn't best practice, but as it's for a university project with a deadline set in concrete and because it appears to work sufficiently well for this purpose I'd rather leave discussing a better way of populating the combo box for another question.
Upvotes: 0
Views: 112
Reputation: 4901
I think there is a solution without instanceof
and cast, if you are using java 7:
ComboBoxModel<Manufacturer> model = new DefaultComboBoxModel<>();
// Initialize model with existing manufacturers model.addElement(...)
JComboBox<Manufacturer> mfctrCombo = new JComboBox<>(model);
// ...
int selectedIndex = mfctrCombo.getSelectedIndex();
Product newPrd = new Product ();
if (selectedIndex == -1) {
// The user provided a string then we need to create a new Manufacturer
Manufacturer newMfr = new Manufacturer ();
newMfr.setName (mfctrCombo.getSelectedItem().toString());
// Logic for persisting the new Manufacturer goes here
newPrd.setManufacturer (newMfr);
} else {
ComboBoxModel<Manufacturer> model = mfctrCombo.getModel();
newPrd.setManufacturer (model.getElementAt(selectedIndex);
}
Furthermore, you don't need to rely on toString()
for display text on GUI (toString()
is not meant to be used that way, it would be problematic in case of internationalization), you can provide a ListCellRenderer instead, as shown in question JComboBox setting label and value.
Upvotes: 0
Reputation: 12196
You still have to cast the instance, however testing with instanceof
first means the compiler won't raise an unchecked cast warning and also means you won't be surprised at runtime with an java.lang.ClassCastException
if the object turns out to be something you didn't expect.
Upvotes: 1
Reputation: 16987
Yes, it is required. This makes it easier for the programmers who had to implement the java compiler. If this feature was to be implemented, they would have to go through a large amount of agony, writing code to find out if a particular object was guaranteed to be a particular type through if statements, which nobody would ever want to do, and the java compiler would remain perfectly functional without it. Plus, probably nobody thought of it.
So, sorry, there is no sane way to do this without a cast.
Upvotes: 1
Reputation: 2155
If it is not an instance of String, it will not go to the if block only. So you don't need to cast in if part of your code.
Upvotes: 0