[Webinar] Streamline your web hosting managementRegister Today

x
  • Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 241
  • Last Modified:

Java Replace Nested If statements with 2-D Map

Playing around again with adding Units to a Java program. Trying another approach, I ended up with a monolithic method, which the code is so ugly I'll post it lower down, and just describe here the simple idea of what it does.

It's basically a huge nested if-else statement, which determines which units we currently have and what units we want to convert to, and then calling the appropriate conversion.

For my test example I have two Measurement types: Temperature and Length.

Temperature measurements have 3 units to choose from: Fahrenheit, Celsius, and Kelvin.

Length measurements have 3 units to choose from: Inches, Feet, Miles.

So basically I first determine if this is a Temperature measurement or a Length measurement.

For a Temperature measurement the possibilities are:

Fahrenheit → Fahrenheit
Fahrenheit → Celsius
Fahrenheit → Kelvin
Celsius → Fahrenheit
Celsius → Celsius
Celsius → Kelvin
Kelvin → Fahrenheit
Kelvin → Celsius
Kelvin → Kelvin

(Note how this looks like an Array.)
           Fahrenheit Celsius Kelvin
          +----------+-------+-------+
Fahrenheit|          |       |       |
          +----------+-------+-------+
Celsius   |          |       |       |
          +----------+-------+-------+
Kelvin    |          |       |       |
          +----------+-------+-------+

Open in new window


For a Length measurement the possibilities are:

Inches → Inches
Inches → Feet
Inches → Miles
Feet → Inches
Feet → Feet
Feet → Miles
Miles → Inches
Miles → Feet
Miles → Miles

As you can imagine this leads to a very ugly nested if-else statement which while in theory it works, I'm looking for a nicer alternative.


 that looks like this:
	public double getValueAs(T targetUnits) {

		// TEMPERATURE MEASUREMENT *******************************************
		if (this.measurementUnits.getClass() == TemperatureUnits.class) {
			
			if (targetUnits == TemperatureUnits.FAHRENHEIT) {
				if (this.measurementUnits == TemperatureUnits.FAHRENHEIT) {
					return this.value;
				}
				else if (this.measurementUnits == TemperatureUnits.CELSIUS) {
					return convertCelsiusToFahrenheit(this.value);
				}
				else if (this.measurementUnits == TemperatureUnits.KELVIN) {
					return convertKelvinToFahrenheit(this.value);
				}
				else {
					//can not convert
				}
			}
			else if (targetUnits == TemperatureUnits.CELSIUS) {
				if (this.measurementUnits == TemperatureUnits.FAHRENHEIT) {
					return convertFahrenheitToCelsius(this.value);
				}
				else if (this.measurementUnits == TemperatureUnits.CELSIUS) {
					return this.value;
				}
				else if (this.measurementUnits == TemperatureUnits.KELVIN) {
					return convertKelvinToCelsius(this.value);
				}
				else {
					//can not convert
				}
				
			}
			else if (targetUnits == TemperatureUnits.KELVIN) {
				if (this.measurementUnits == TemperatureUnits.FAHRENHEIT) {
					return convertFahrenheitToKelvin(this.value);
				}
				else if (this.measurementUnits == TemperatureUnits.CELSIUS) {
					return convertCelsiusToKelvin(this.value);
				}
				else if (this.measurementUnits == TemperatureUnits.KELVIN) {
					return value;
				}
				else {
					//can not convert
				}
				
			}
			else {
				//can not convert
			}
		}

		// LENGTH MEASUREMENT *******************************************
		else if (this.unitType == LengthUnits.class) {
			if (targetUnits == LengthUnits.INCHES) {
			
				if (this.measurementUnits == LengthUnits.INCHES) {
					return this.value;
				}
				else if (this.measurementUnits == LengthUnits.FEET) {
					return convertFeetToInches(this.value);
				}
				else if (this.measurementUnits == LengthUnits.MILES) {
					return convertMilesToInches(this.value);
				}
				else {
					//can not convert
				}
			}
			else if (targetUnits == LengthUnits.FEET) {
				if (this.measurementUnits == LengthUnits.INCHES) {
					return convertInchesToFeet(this.value);
				}
				else if (this.measurementUnits == LengthUnits.FEET) {
					return value;
				}
				else if (this.measurementUnits == LengthUnits.MILES) {
					return convertMilesToFeet(this.value);
				}
				else {
					//can not convert
				}				
			}
			else if (targetUnits == LengthUnits.MILES) {
				if (this.measurementUnits == LengthUnits.INCHES) {
					return convertInchesToMiles(this.value);
				}
				else if (this.measurementUnits == LengthUnits.FEET) {
					return convertFeetToMiles(this.value);
				}
				else if (this.measurementUnits == LengthUnits.MILES) {
					return value;
				}
				else {
					//can not convert
				}
			
			}
			else {
				//can not convert
			}

		}
	
		return 0;
	}

Open in new window

My first attempt was to try Polymorphism and Strategy Pattern, but I realized that was just spreading out the if-else statements so they were all over the place, rather than reducing them or eliminating them.

Next (and current) idea is to create a HashMap, with an array of keys for the units (from this unit to that unit), and the value will be the appropriate conversion method (I'm expecting something implementing the callable interface).

My units are enums: Temperature={FAHRENHEIT, CELSIUS, KELVIN}, Length={INCHES, FEET, MILES}

Now I'm wondering how to implement this Map of Array → Values. I can think of two possibilities:

1. Somehow flatten my 2-D Array of enum Keys to a 1-D array of Keys, and then use HashMap.

OR

2. Create a nested tree of HashMaps.

(Oh yes, and also "Create an Interface so the implementation details can change later on if I come up with a better way.")
Any ideas or suggestions? Preference for idea #1 or #2? Perhaps a #3 idea I haven't thought of yet?
0
deleyd
Asked:
deleyd
  • 2
  • 2
  • 2
1 Solution
 
dpearsonCommented:
You could do it with a map and a set of Callable objects, but it doesn't scale well.

The problem is that your basic approach is quadratic in the number of units.  You have 3 possible inputs and 3 possible outputs, so there are 9 cells (3x3) in your matrix.  But if you expand this to 6 units, you get 36 cells in your matrix, each of which needs the correct code (the correct Callable).  It's the sort of design that leads to problems - because it's easy to get it right for 33 of those 36 and wrong for the last 3 and then that's very hard to find.  (Why does converting to Farenheit work from Kelvin, but not from Celsius? etc.)

What I'd propose instead is reducing the complexity to a linear relationship with your number of units.  You can do that by mapping from an arbitrary input unit to one standard unit and then from that standard unit back to an arbitrary unit.  That way if you have 6 units, there are 6 mappings to the standard and then 6 back.  Which is linear rather than quadratic, so much easier to test and get right.

Let me make that a bit more concrete with some code...which I hope you can see is simpler, because each method is pretty short and easy to understand.  Also if you wish to add a new set of units, the places to put the new conversion are pretty obvious.

Any good?
 
Doug

public double getValueAs(T targetUnits) {
    if (this.measurementUnits.getClass() == TemperatureUnits.class) {
        double celsius = convertToCelsius(this.measureUnits, this.value) ;
        double converted = convertFromCelsius(targetUnits, celsius) ;
        return converted ;
    }

   // Same approach but for length units - convert them all to meters and then back to actual target
}

double static convertToCelsius(T fromUnits, double val) {
   switch (fromUnits) {
        case TemperatureUnits.FAHRENHEIT: return (val - 32) * 5.0 / 9.0 ;
        case TemperatureUnits.CELSIUS: return this.val ;
        case TemperatureUnits.KELVIN: return this.val + 273 ;
        default: throw new IllegalArgument("Unknown units " + fromUnits) ;
   }
}

double static convertFromCelsius(T toUnits, double val) {
   switch (toUnits) {
        case TemperatureUnits.FAHRENHEIT: return (val * 9.0 / 5.0) + 32.0 ;
        case TemperatureUnits.CELSIUS: return this.val ;
        case TemperatureUnits.KELVIN: return this.val - 273 ;
        default: throw new IllegalArgument("Unknown units " + toUnits) ;
   }
}

Open in new window

0
 
deleydAuthor Commented:
Oh yes that is a good idea. I has the same idea and noticed JScience did the same thing: convert to a base unit, then from there convert to targetUnit. (Though I noticed if I only have 2 units then I don't save much doing it that way.)

My next thought was my Measurement<T> class is going to get very cluttered if there are a lot of Measurement Types (Temperature, Length,..) and each measurement type has numerous units to choose from.

Is there a way to extract some of this conversion code into it's own class without using a cast? I tried making a TemperatureConverter class, but found I had to cast which always makes me a bit wary:
	public double getValueAs(T targetUnits) {
	    if (this.measurementUnits.getClass() == TemperatureUnits.class) {
	    	return TemperatureConverter.convert((TemperatureUnits) this.measurementUnits, (TemperatureUnits) targetUnits, this.value);
	    }

Open in new window

(plus I haven't actually tested the targetUnits to see if I can cast them to TemperatureUnits. I'll have to throw in a test for that if casting is the way to go.)

Is there a nicer way to code this?
0
 
mccarlIT Business Systems Analyst / Software DeveloperCommented:
Oh yes that is a good idea. I has the same idea and noticed JScience did the same thing: convert to a base unit, then from there convert to targetUnit.
Is there a way to extract some of this conversion code into it's own class without using a cast?

I've already given you a solution for this. Did you not read these posts that we took the time to provide to you?
0
The new generation of project management tools

With monday.com’s project management tool, you can see what everyone on your team is working in a single glance. Its intuitive dashboards are customizable, so you can create systems that work for you.

 
dpearsonCommented:
Nice - I like how mccarl suggested the same solution to the conversion problem I just did - i.e. going to a standard unit first.  (Just as one part of what he's laying out over there).

Great minds think alike :)

Doug

P.S. As for the follow-up question, you could always just go with
public double getTemperatureValueAs(TemperatureUnit targetUnits) {...}
and you should find no casts required now.

In general I think you're struggling between trying to make everything super general and then later on trying to impose type safety.  Easier to just start type safe if you can.
0
 
mccarlIT Business Systems Analyst / Software DeveloperCommented:
Great minds think alike :)
Indeed ! ;)
0
 
deleydAuthor Commented:
I've ultimately decided using Generics it's just not possible to achieve what I'm after, because I came to a situation where I wanted to pass the type of enum {Temperature, Length,...}
public class Measurement<T extends Units> {

	private Class<T> unitType;
	private T measurementUnits;
	private double value;
	
	public Measurement(T u, double v) {
		this.value = v;
		this.measurementUnits = u;
		this.unitType = u.getClass();  // CAN'T DO THIS
		T[] x = u.values();   // CAN'T DO THIS
	}

Open in new window

So I'm switching back to my original idea of generating massive amounts of nearly identical code using a Code Template and a simple code generating program which replaces "MeasurementType" with "Temperature" or "Length" or whatever...
0

Featured Post

Free Tool: Port Scanner

Check which ports are open to the outside world. Helps make sure that your firewall rules are working as intended.

One of a set of tools we are providing to everyone as a way of saying thank you for being a part of the community.

  • 2
  • 2
  • 2
Tackle projects and never again get stuck behind a technical roadblock.
Join Now