Solved

Java Replace Nested If statements with 2-D Map

Posted on 2014-10-04
6
209 Views
Last Modified: 2014-10-14
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
Comment
Question by:deleyd
  • 2
  • 2
  • 2
6 Comments
 
LVL 26

Accepted Solution

by:
dpearson earned 500 total points
ID: 40361786
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
 

Author Comment

by:deleyd
ID: 40363060
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
 
LVL 35

Expert Comment

by:mccarl
ID: 40367448
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
Highfive Gives IT Their Time Back

Highfive is so simple that setting up every meeting room takes just minutes and every employee will be able to start or join a call from any room with ease. Never be called into a meeting just to get it started again. This is how video conferencing should work!

 
LVL 26

Expert Comment

by:dpearson
ID: 40367458
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
 
LVL 35

Expert Comment

by:mccarl
ID: 40367609
Great minds think alike :)
Indeed ! ;)
0
 

Author Closing Comment

by:deleyd
ID: 40381418
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

How to run any project with ease

Manage projects of all sizes how you want. Great for personal to-do lists, project milestones, team priorities and launch plans.
- Combine task lists, docs, spreadsheets, and chat in one
- View and edit from mobile/offline
- Cut down on emails

Join & Write a Comment

Suggested Solutions

Title # Comments Views Activity
countPairs challenge 7 58
word0 challenge 3 58
Running Jira on Raspberry PI 2? 3 127
Java JRE greater than 1.6 5 19
This was posted to the Netbeans forum a Feb, 2010 and I also sent it to Verisign. Who didn't help much in my struggles to get my application signed. ------------------------- Start The idea here is to target your cell phones with the correct…
Go is an acronym of golang, is a programming language developed Google in 2007. Go is a new language that is mostly in the C family, with significant input from Pascal/Modula/Oberon family. Hence Go arisen as low-level language with fast compilation…
Video by: Michael
Viewers learn about how to reduce the potential repetitiveness of coding in main by developing methods to perform specific tasks for their program. Additionally, objects are introduced for the purpose of learning how to call methods in Java. Define …
This tutorial covers a practical example of lazy loading technique and early loading technique in a Singleton Design Pattern.

705 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

19 Experts available now in Live!

Get 1:1 Help Now