Reducing IF statement java

Question

i am trying to make a unit converter using javafx - i've been searching around for two days on how can i reduces these if-statements. somehow, i found some similar issue but it didn't help me since i am new with Java - i do not know the right approach in my case.

Hope you guys could help me -

thanks

/**
     * CELSIUS TO
     */
    Celsius celsius = new Celsius(parseInput);
        if(cbOne.getValue().equals("Celsius")) {
            if(cbTwo.getValue().equals("Celsius") ) {
                showAnswer.setText(celsius.celsiusToCelsius());
            }
            if(cbTwo.getValue().equals("Fahrenheit")) {
                showAnswer.setText(celsius.celsiusToFahrenheit());
            }
            if(cbTwo.getValue().equals("Kelvin")) {
                showAnswer.setText(celsius.celsiusToKelvin());
            }
        }
    /**
     * FAHRENHEIT TO
     */
    Fahrenheit fahr = new Fahrenheit(parseInput);
        if(cbOne.getValue().equals("Fahrenheit") ) {
            if(cbTwo.getValue().equals("Celsius") ) {
                showAnswer.setText(fahr.fahrenheitToCelsius());
            }
            if(cbTwo.getValue().equals("Fahrenheit")) {
                showAnswer.setText(fahr.fahrenheitToFahrenheit());
            }
            if(cbTwo.getValue().equals("Kelvin")) {
                showAnswer.setText(fahr.fahrenheitToKelvin());
            }
        }
    /**
     * KELVIN TO
     */
    Kelvin kelvin = new Kelvin(parseInput);
        if(cbOne.getValue().equals("Kelvin")) {
            if(cbTwo.getValue().equals("Celsius") ) {
                showAnswer.setText(kelvin.kelvinToCelsius());
            }
            if(cbTwo.getValue().equals("Fahrenheit")) {
                showAnswer.setText(kelvin.kelvinToFahrenheit());
            }
            if(cbTwo.getValue().equals("Kelvin")) {
                showAnswer.setText(kelvin.kelvinToKelvin());
            }
        }
    }

Answer 1

Actually your if statements are fine for a small program. They are perfectly clear. You may, however, reduce redundancy by checking that cbOne.getValue().equals(cbTwo.getValue()). This will trade 3 of your if statements for 1.

If you had lots of these, you would benefit from a Map and interface scheme.

interface Converter {
    double convert(double from);
}

static final Map<String, Map<String, Converter>> converters = (
    new HashMap<String, Map<String, Converter>>()
);
static {
    Map<String, Converter> fromCelsius = new HashMap<String, Converter>();

    fromCelsius.put(   "Celsius", new NoConversionConverter()       );
    fromCelsius.put("Fahrenheit", new CelsiusToFahrenheitConverter());
    fromCelsius.put(    "Kelvin", new CelsiusToKelvinConverter()    );

    converters.put("Celsius", fromCelsius);

    ...
}

static Converter getConverter(String from, String to) {
    Map<String, Converter> fromMap = converters.get(from);
    return fromMap == null ? null : fromMap.get(to);
}

A Map is the common OOP solution. Instead of imperative/structured decision-making we configure the Map and the decision-making is hidden by abstraction.

This is extremely succinct in Java 8 when paired with an enum:

public enum Scale {
    CELSIUS, FAHRENHEIT, KELVIN;
    private final Map<Scale, DoubleUnaryOperator> ops = new HashMap<>();

    public DoubleUnaryOperator to(Scale to) {
        return to == this ? DoubleUnaryOperator.identity() : ops.get(to);
    }

    static {
        put(    CELSIUS, FAHRENHEIT, c -> c * 9.0 / 5.0 + 32.0     );
        put(    CELSIUS,     KELVIN, c -> c + 273.15               );
        put( FAHRENHEIT,    CELSIUS, f -> (f - 32.0) * 5.0 / 9.0   );
        put( FAHRENHEIT,     KELVIN, f -> (f + 459.67) * 5.0 / 9.0 );
        put(     KELVIN, FAHRENHEIT, k -> k * 9.0 / 5.0 + 459.67   );
        put(     KELVIN,    CELSIUS, k -> k - 273.15               );
    }

    private static void put(Scale from, Scale to, DoubleUnaryOperator op) {
        from.ops.put(to, op);
    }
}

Also extremely readable:

Scale      source = Scale.valueOf("CELSIUS");
Scale destination = Scale.valueOf("FAHRENHEIT");
double     result = source.to(destination).applyAsDouble(0.0);

Answer 2

You do not need 3 classes to represent temperatures in different scales. Create a class that always keeps temperature in Kelvins internally and can convert it to any other scale for output. Having a class like this:

public final class Temperature {
  public enum Scale {
    Celsius, Fahrenheit, Kelvin
  }

  private final double temperature;

  private Temperature(double temperature) {
    this.temperature = temperature;
  }

  public static Temperature create(double temperature, Scale scale) {
    switch (scale) {
      case Celsius:
        return new Temperature(temperature + 273.15);
      case Fahrenheit:
        return new Temperature((temperature + 459.67) * 5.0 / 9.0);
      case Kelvin:
        return new Temperature(temperature);
      default:
        throw new IllegalArgumentException("Unknown scale");
    }
  }

  public double convertTo(Scale scale) {
    switch (scale) {
      case Celsius:
        return temperature - 273.15;
      case Fahrenheit:
        return temperature * 9.0 / 5.0 - 459.67;
      case Kelvin:
        return temperature;
      default:
        throw new IllegalArgumentException("Unknown scale");
    }
  }
}

Your code becomes:

Temperature temp = Temperature.create(parseInput, Scale.valueOf(cbOne.getValue()));
showAnswer.setText(temp.convertTo(Scale.valueOf(cbTwo.getValue())));

Answer 3

Your biggest issue is that you have created an n×m problem that ought to be a n+m problem.

To solves this, first define a canonical unit and dissolve the problem into two steps, convert from the source unit to the canonical unit, then convert from the canonical unit to the target unit.

For example, if you define kelvin to be the canonical unit, your code might resemble this:

switch(inputUnit.getValue())
{
  case "Fahrenheit": kelvin=fahrenheitToKelvin(input); break;
  case "Celsius":    kelvin=celsiusToKelvin(input); break;
  case "Kelvin":     kelvin=input; break;
  default: throw new AssertionError();
}
switch(outputUnit.getValue())
{
  case "Fahrenheit": output=kelvinToFahrenheit(kelvin); break;
  case "Celsius":    output=kelvinToCelsius(kelvin); break;
  case "Kelvin":     output=kelvin; break;
  default: throw new AssertionError();
}
showAnswer.setText(output);

I omitted string to number and number to string conversions as it should be obvious that these conversions need to be performed only once, outside the selector.

This principle can be used as well, if you use enums instead of Strings or replace the switch with a Map based approach as suggested by others. But the important thing is the two-step approach that allows you to maintain n input unit to canonical plus m canonical to output unit conversions rather than n input units times m output units conversions.

Answer 4

Since you are not going to have to modify this code later with more units, I would suggest you a ternary operator :

String s = cbTwo.getValue();
showAnswer.setText(s.equals("Celsius") ? fahr.fahrenheitToCelsius() :
    s.equals("Farenheit") ? fahr.fahrenheitToFahrenheit() : kelvin.kelvinToKelvin()); 

Note that it is not exactly equivalent, in the case that s does not match any of thestrings you have in your if statements.

Answer 5

You can convert any input value to Kelvin and then convert from Kelvin to the desired result:

String unit = cbOne.getValue();
double inputInKelvin;
String outUnit = cbTwo.getValue();

// parse
if ( unit.equals("Celsius") ) inputInKelvin = new Celsius(parseInput).celsiusToKelvin();
else if ( unit.equals("Fahrenheit") ) inputInKelvin = new Fahrenheit(parseInput).fahrenheitToKelvin();
else inputInKelvin = new Kelvin(parseInput).kelvinToKelvin();

// output
Kelvin kelvin = new Kelvin(inputInKelvin);
if ( unit.equals("Celsius") ) showAnswer.setText(kelvin.kelvinToCelsius());
else if ( unit.equals("Fahrenheit") ) showAnswer.setText(kelvin.kelvinToFahrenheit());
else showAnswer.setText( kelvin.kelvinToKelvin() );

It would get even more readable if you parse the String to a double first and then just have one Converter class.

Answer 6

While a ternary operator would work, it's difficult to read and maintain.

Slightly better could be a switch statement within the first if statement. However, your code as written is adequately understandable and doesn't need to change from that perspective.

However, to really make this code better, it needs to be re-factored.

There are two troublesome characteristics of the code as written. First, you instantiate all temperature objects when only one is used.

Second, the UI and the business logic are too tightly coupled. if you change the name of the UI elements, you'll have to change the business logic. Also, if you add additional ways for your application to convert temperature, you won't be able to re-use this code.

Answer 7

You can also do something like this:

 final int CELSIUS = 1,FARANHITE = 2,KELVIN = 3;

then use switch statment like this

 int key = StringToInt(firstValue)*10 + StringTOInt(secondValue);
 //This will give you 9 unique codes 11 12 13 21 22 23 ....
 switch(key)
 {
       default: case 11: case 22: case 33: break;
       //do nothing since no conversion required

       case 12://Celsius to faranhite
       case 13://celsius to kelvin
       .
       .
       //and so on     

 }

Answer 8

Do like this

create an InterfaceForConvertorTypes

public interface Convertor {
    public String convert(int parseInt);
    public boolean accept(String from, String to);
}

then implement convertor types and register them in registery collection. with a single IF you can achieve what you want.

List<Convertor> convertors = new ArrayList<Convertor>();

    Convertor CelsiusToCelsius = new Convertor() {

        @Override
        public String convert(int parseInt) {
            Celsius celsius = new Celsius(parseInt);
            return celsius.celsiusToCelsius();
        }

        @Override
        public boolean accept(String from, String to) {
            return from.equals("Celsius") && to.equals("Celsius");
        }
    };


    Convertor CelsiusToFah = new Convertor() {

        @Override
        public String convert(int parseInt) {
            Celsius celsius = new Celsius(parseInt);
            return celsius.celsiusToFahrenheit();
        }

        @Override
        public boolean accept(String from, String to) {
            return from.equals("Celsius") && to.equals("Fahrenheit");
        }
    };

    Convertor CelsiusToKelvin = new Convertor() {

        @Override
        public String convert(int parseInt) {
            Celsius celsius = new Celsius(parseInt);
            return celsius.celsiusToFahrenheit();
        }

        @Override
        public boolean accept(String from, String to) {
            return from.equals("Celsius") && to.equals("Kelvin");
        }
    };

   // create Rest of Convertor like above

    convertors.add(CelsiusToFah);
    convertors.add(CelsiusToCelsius);
    convertors.add(CelsiusToKelvin);
    // register rest of convertor


    //Thats it!
    for(Convertor convertor:convertors) {
        if(convertor.accept(cbOne.getValue(), cbTwo.getValue())) {
            showAnswer.setText(convertor.convert(parseInput));
        }
    }