I'm wondering if it's possible to use switch statement instead of if-else one in this case. Variables are taken from JComboBoxes and processed by ActionEvent here:
public void actionPerformed(ActionEvent event) {
Object source = event.getSource();
if (source == comboUnit) {
String unit = comboUnit.getSelectedItem().toString();
if (unit.equals("Unit 1")) {
unitValue = Double.parseDouble(tfUnit.getText());
valMeter = unitValue * defined1;
labelDesc.setText("Unit 1");
convert();
}
else if (unit.equals("Unit 2")) {
unitValue = Double.parseDouble(tfUnit.getText());
valMeter = unitValue * defined2;
labelDesc.setText("Unit 2");
convert();
}
(...)
I was trying to pass pure strings as the values but with no success. Do you have any hints on how it should be done (and if it's even possible)?
Java 7 allows you to switch
on String
s.
Your code needs refactoring though, and the question of whether to use switch
or enumerations or maps, is something that comes second, I would say.
You have too much duplicated code doing essentially the same thing. I am referring to:
unitValue = Double.parseDouble(tfUnit.getText());
valMeter = unitValue * defined1;
labelDesc.setText("Unit 1");
convert();
Obviously, because you multiply using different factors depending on the unit used, you need a function of unit evaluating to the factor to use. In less mathematical terms, you need something that yields value of either defined1
or defined2
depending on a string supplied. You already have the unit
referring to the "name" of your unit in question, you can use it. I call the method returning the factor for factor
, taking in a unit name and returning a Number
(since it does not follow from your example whether you are multiplying integers or real numbers of some sort). I am also assuming your defined1
and defined2
etc, are variables or literals.
unitValue = Double.parseDouble(tfUnit.getText());
valMeter = unitValue * factor(unit);
labelDesc.setText(unit);
convert();
Number factor(String unitName) {
switch(unitName) {
case "Unit 1": return defined1;
case "Unit 2": return defined2;
default: throw new Exception("Unknown unit");
}
}
The method itself is where your "to switch or not to switch" problem creeps in. You are free to use a map if you want:
Map<String, Number> unitNameValueMap = new HashMap<String, Number>();
unitNameValueMap.put("Unit 1", defined1);
unitNameValueMap.put("Unit 2", defined2);
Number factor(String unitName) {
Number result = unitNameValueMap.get(unitName);
if(result == null) throw new Exception("Unknown unit");
return result;
}
Or you can use enumerations:
enum UnitValue {
UNIT1(defined1), UNIT2(defined2);
final Number value;
private UnitValue(Number value) {
this.value = value;
}
}
Number factor(String unitName) {
return Enum.valueOf(UnitValue.class, "UNIT" + Integer.parseInt(unitName.substring(5)).value;
}
You can even switch
or use a map inside enumerations, which will give you good code readability as well.
You need to profile your program to see if you need switch
-based solution or enum-based one or a map-based one. The way it currently stands and as you can see for yourself, the enum
-based solution is a bit messy because of relationship between your unit names, enumeration constants, and their values. If someone can do better with enums, then take their code instead, naturally.
Usually, shortest code is best, as it is easier to read and most often understand. But be careful with maps - they induce more overhead than other solutions, so I for one prefer to use them where I have fewer maps with many keys each, not the other way around.