GordonM
GordonM

Reputation: 31730

Casting known object types

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

Answers (4)

Katona
Katona

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

Dev
Dev

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

tbodt
tbodt

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

Crickcoder
Crickcoder

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

Related Questions