Conversion Between Units

Question

I have created a java program that should take a list of conversions and be able to convert one unit to another. My main goals are maintainability and readability, but speed would be nice as well.

Example input.in

1 meters = 100 centimeters
10 millimeters = 1 centimeters
1 meters = 0.001 kilometers
1 kilometers = 1000000000000 nanometers
10 dekameters = 1 hectometers

Example input to the program:

Enter the first unit: meters
Enter the amount of the first unit: 12
Enter the unit to convert to: nanometers

Example output: 12000000000.0 or 1.2e10

ConversionsMain.java

package conversions;
import java.io.BufferedReader;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStreamReader;
import java.util.Scanner;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import conversions.ConversionExceptions.*;
public class ConversionsMain {
 private static BufferedReader inputReader;
 private static Scanner userInput;
 public static final String INPUT_FILE_NAME = "input.in";
 // Used to validate numbers
 // Should match: 1, 20, 57, 88.6, 88.60, 0.75
 // Shouldn't match: not_number, .75, 9., 088
 private static final String numberRegex = "([1-9][0-9]*(?:\\.[0-9]*)?|0\\.[0-9]*)",
 // Validates a unit of measurement
 unitRegex = "([a-zA-Z]+)";
 // An entire line consists of number unit = number unit e.g. 100centimeters=1meter
 private static final Pattern tokenPattern = Pattern.compile(
 "^" + numberRegex + unitRegex + "=" + numberRegex + unitRegex + "$"
 );
 private ConversionMap conversionMap = new ConversionMap();
 public static void main(String[] args) {
 ConversionsMain main = new ConversionsMain();
 try {
 inputReader = new BufferedReader(new InputStreamReader(new FileInputStream(INPUT_FILE_NAME)));
 userInput = new Scanner(System.in);
 main.start();
 } catch (IOException e) {
 System.out.println("An error occured with the file.\nException details:");
 e.printStackTrace();
 } catch (MalformedLineException | NumberFormatException e) {
 System.out.println("The line does not conform to the standard.\nException details:");
 e.printStackTrace();
 } catch (NoSuchUnitException e) {
 System.out.println("The input is not valid because one more more units was missing.\nDetails:");
 e.printStackTrace();
 } catch (NoPathBetweenUnits e) {
 System.out.println("It is not possible to convert between the two units.");
 e.printStackTrace();
 } finally {
 try {
 userInput.close();
 inputReader.close();
 } catch (IOException e) {
 System.out.println("An error occured with closing the file.\nException details:");
 e.printStackTrace();
 }
 }
 }
 public void start() throws MalformedLineException, IOException, NumberFormatException, NoSuchUnitException, NoPathBetweenUnits {
 // Parse every line
 for (String line = inputReader.readLine(); line != null; line = inputReader.readLine()) {
 parseLine(line);
 }
 // Convert
 System.out.println("Enter the first unit");
 String unit1 = userInput.next(unitRegex);
 System.out.println("Enter the amount of the first unit");
 double unit1Amount = userInput.nextDouble();
 System.out.println("Enter the unit to convert to");
 String unit2 = userInput.next(unitRegex);
 System.out.println(conversionMap.convert(unit1Amount, unit1, unit2));
 }
 private void parseLine(String line) throws MalformedLineException, NumberFormatException {
 // Remove spaces
 line = line.replaceAll(" ", "");
 Matcher matcher = tokenPattern.matcher(line);
 // Make sure that the line matches the acceptable format
 if (!matcher.matches()) {
 throw new MalformedLineException("The line " + line + " does not conform to the regex.");
 }
 // Parse the numbers before the units
 double number1 = Double.parseDouble(matcher.group(1)),
 number2 = Double.parseDouble(matcher.group(3));
 // The two units
 String item1 = matcher.group(2),
 item2 = matcher.group(4);
 // Add a conversion relationship between these two
 conversionMap.addConversion(item1, number1, item2, number2);
 }
}

ConversionMap.java

package conversions;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import conversions.ConversionExceptions.*;
public class ConversionMap {
 /**
 * A piece of data with a unit (string) and a unit amount
 */
 private static class Conversion {
 private String unitName;
 private double unitAmount;
 /**
 * Constructs a conversion with the given unit name and amount
 * @param unitName The name of the unit
 * @param unitAmount The amount of the unit that should be associated with the unit
 */
 public Conversion(String unitName, double unitAmount) {
 this.unitName = unitName;
 this.unitAmount = unitAmount;
 }
 public String toString() {
 return unitAmount + " " + getUnitName();
 }
 /**
 * 
 * @return The name of the unit
 */
 public String getUnitName() {
 return unitName;
 }
 public boolean equals(Object o) {
 if (o == this) return true;
 if (o.getClass() != this.getClass()) return false;
 Conversion other = (Conversion) o;
 return other.unitName.equals(unitName) && other.unitAmount == unitAmount;
 }
 public int hashCode() {
 final int prime = 31;
 long bitsOfUnitAmount = Double.doubleToLongBits(unitAmount);
 return ((int) (bitsOfUnitAmount ^ (bitsOfUnitAmount >>> 32))) * prime + unitName.hashCode();
 }
 }
 // A map of units to a set of conversions they could have
 // Example possibility:
 // {"yards": Set[Conversion(3 "feet"), Conversion(36, "inches")]}
 private Map<String, Set<Conversion>> conversionMap = new HashMap<String, Set<Conversion>>();
 /**
 * Adds a conversion rule by mapping a unit to another unit
 * Example: addConversion("meters", 1000, "kilometers", 1)
 * @param unit1 The first unit
 * @param amount1 The amount of the first unit
 * @param unit2 The second unit
 * @param amount2 The amount of the second unit
 */
 public void addConversion(String unit1, double amount1, String unit2, double amount2) {
 // Create the two conversions
 Conversion amountUnit1InUnit2 = new Conversion(unit2, amount2 / amount1),
 amountUnit2InUnit1 = new Conversion(unit1, amount1 / amount2);
 Set<Conversion> conversionsUnit1 = conversionMap.get(unit1),
 conversionsUnit2 = conversionMap.get(unit2);
 // If either unit has not been seen before, then create a set and add the unit
 if (conversionsUnit1 == null) {
 conversionsUnit1 = new HashSet<Conversion>();
 conversionMap.put(unit1, conversionsUnit1);
 }
 if (conversionsUnit2 == null) {
 conversionsUnit2 = new HashSet<Conversion>();
 conversionMap.put(unit2, conversionsUnit2);
 }
 conversionsUnit1.add(amountUnit1InUnit2);
 conversionsUnit2.add(amountUnit2InUnit1);
 }
 /**
 * Gets the shortest path that can be used to convert from one unit to another
 * @param unit1 The first unit
 * @param unit2 The second unit (destination unit)
 * @param visited A map consisting of the shortest known paths to each unit we've visited so far
 * @return One of the shortest conversion paths that can be used to convert between units or null if no such path can be found
 */
 public ConversionPath getConversionPath(String unit1, String unit2, Map<String, ConversionPath> visited) {
 // Get the currentPath used to get to the current unit
 ConversionPath currentPath = visited.get(unit1);
 // If unit1 and unit2 are the same, then we've reached the destination, return the path so far
 if (unit1.equals(unit2)) {
 return currentPath;
 }
 // Find all possible conversions one level deep from this unit
 Set<Conversion> possibleConversions = conversionMap.get(unit1);
 int minLength = Integer.MAX_VALUE;
 ConversionPath smallestPath = null;
 // Loop through each conversion
 for (Conversion conversion : possibleConversions) {
 ConversionPath path = visited.get(conversion.getUnitName());
 // If going to the unit through this unit is shorter than what was previously there
 // then search for the destination from that unit
 if (path == null || path.pathLength() > currentPath.pathLength() + 1) {
 // Update the path on the other unit, because there is now a shorter way to get there
 visited.put(conversion.getUnitName(), currentPath.addUnit(conversion.getUnitName()));
 // Get the path from the new unit to the destination unit
 ConversionPath newPath = getConversionPath(conversion.getUnitName(), unit2, visited);
 // If such a path exists and its length is less than the shortest length so far
 // update the shortest path and smallest length
 if (newPath != null && newPath.pathLength() < minLength) {
 minLength = newPath.pathLength();
 smallestPath = newPath;
 }
 }
 }
 return smallestPath;
 }
 /**
 * Obtains the shortest path used to convert between two units
 * @param unit1 The starting unit
 * @param unit2 The destination unit
 * @return A ConversionPath representing the shortest path to convert from unit1 to unit2
 */
 public ConversionPath getConversionPath(String unit1, String unit2) {
 ConversionPath path = new ConversionPath();
 Map<String, ConversionPath> visited = new HashMap<String, ConversionPath>();
 visited.put(unit1, path);
 return getConversionPath(unit1, unit2, visited);
 }
 /**
 * Converts between two units.
 * Example: convert(10.0, "years", "days") --> returns 3650
 * @param unit1Amount The amount of unit1 that there is
 * @param unit1 The starting unit
 * @param unit2 The destination unit
 * @return The amount of unit2 there is when unit1 is converted to unit2
 * @throws NoSuchUnitException If either unit1 or unit2 do not exist
 * @throws NoPathBetweenUnits If unit1 or unit2 exist but there is no way to convert between them
 */
 public double convert(double unit1Amount, String unit1, String unit2) throws NoSuchUnitException, NoPathBetweenUnits {
 if (!(conversionMap.containsKey(unit1) && conversionMap.containsKey(unit2))) {
 throw new NoSuchUnitException("One of those units does not exist");
 }
 // Get the shortest path
 ConversionPath path = getConversionPath(unit1, unit2);
 if (path == null) {
 throw new NoPathBetweenUnits("Could not find a path between " + unit1 + " and " + unit2);
 }
 String previousUnit = unit1;
 // Loop through each unit in the path
 for (String unit : path.units) {
 // Try to convert previousUnit to unit
 // Get the possible conversions one could make with the previous unit
 Set<Conversion> conversions = conversionMap.get(previousUnit);
 // Find the amount of previousUnit it takes to make unit
 for (Conversion conversion : conversions) {
 if (conversion.getUnitName().equals(unit)) {
 unit1Amount *= conversion.unitAmount;
 System.out.println(" = " + unit1Amount + " " + conversion.getUnitName());
 break;
 }
 }
 previousUnit = unit;
 }
 return unit1Amount;
 }
 public String toString() {
 return conversionMap.toString();
 }
}

ConversionPath.java

package conversions;
import java.util.Arrays;
/**
 * A wrapper for String[], an array of units.
 * The path is immutable.
 * Example: to convert from seconds to hours the ConversionPath is ["minutes", "hours"]
 */
public class ConversionPath {
 String[] units;
 /**
 * Creates a conversion path with the specified units.
 * @param units The units that this conversion path has
 */
 public ConversionPath(String... units) {
 this.units = units;
 }
 /**
 * Duplicates the array and adds a unit to it.
 * @param unit The unit to add
 * @return A duplicated conversion path with the unit appended at the end
 */
 public ConversionPath addUnit(String unit) {
 String[] newUnits = Arrays.copyOf(units, units.length + 1);
 newUnits[units.length] = unit;
 return new ConversionPath(newUnits);
 }
 /**
 * Returns the length of the path.
 * @return The length of the path
 */
 public int pathLength() {
 return units.length;
 }
 public String toString() {
 return Arrays.toString(units);
 }
}

The ConversionExceptions.java is just a class with the following static classes which extend Exception: NoSuchUnitException, MalformedLineException, and NoPathBetweenUnits.

Answer 1

Don't use double. Binary floating-point cannot exactly represent most decimal numbers, so you would be implicitly rounding a lot of your numbers before you even start. Read What Every Computer Scientist Should Know About Floating-Point Arithmetic for a more in-depth explanation.

A slightly better choice would be BigDecimal, which can represent any arbitrary decimal number. However, it can't represent fractions like 1/3 exactly, so while the input numbers could be represented exactly, you'd still have to round after every division.

For this program, the best numeric representation would be rational numbers. You could use Apache Commons' BigFraction or roll your own. This way you can retain exact results until you need to print output, at which point you can decide on an output format or let the user choose one.

In ConversionsMain , the static members inputReader and userInput and the member variable conversionMap can be made local within main(). This makes it easier to understand each method on ConversionsMain, since each method operates only on its arguments and doesn't refer to any external state. It also makes refactoring easier.

start() does two things. It should be split into two methods. The code for reading the list of conversions can be moved into a different method or even into a factory method on ConversionMap. There should also be separate try/catch blocks for the two methods so you can print relevant error information on exceptions like IOException.

You can eliminate the file-closing boilerplate by using try-with-resources if you're on Java 7 or above.

Here's a version of ConversionsMain incorporating the changes I suggested (except for BigFraction):

public class ConversionsMain {
 public static final String INPUT_FILE_NAME = "input.in";
 // Used to validate numbers
 // Should match: 1, 20, 57, 88.6, 88.60, 0.75
 // Shouldn't match: not_number, .75, 9., 088
 public static final String numberRegex = "([1-9][0-9]*(?:\\.[0-9]*)?|0\\.[0-9]*)";
 public static final String // Validates a unit of measurement
 unitRegex = "([a-zA-Z]+)";
 // An entire line consists of number unit = number unit e.g. 100centimeters=1meter
 private static final Pattern tokenPattern = Pattern.compile(
 "^" + ConversionsMain.numberRegex + ConversionsMain.unitRegex + "=" + ConversionsMain.numberRegex + ConversionsMain.unitRegex + "$"
 );
 public static void main(String[] args) {
 ConversionMap conversionMap;
 try {
 conversionMap = readConversionMapFromFile(INPUT_FILE_NAME);
 } catch (IOException e) {
 System.out.println("An error occured with the file.\nException details:");
 e.printStackTrace();
 return;
 } catch (MalformedLineException | NumberFormatException e) {
 System.out.println("The line does not conform to the standard.\nException details:");
 e.printStackTrace();
 return;
 }
 try(final Scanner userInput = new Scanner(System.in)) {
 start(conversionMap, userInput);
 } catch (IOException e) {
 System.out.println("An error occured with the console.\nException details:");
 e.printStackTrace();
 } catch (MalformedLineException | NumberFormatException e) {
 System.out.println("The line does not conform to the standard.\nException details:");
 e.printStackTrace();
 } catch (NoSuchUnitException e) {
 System.out.println("The input is not valid because one more more units was missing.\nDetails:");
 e.printStackTrace();
 } catch (NoPathBetweenUnits e) {
 System.out.println("It is not possible to convert between the two units.");
 e.printStackTrace();
 }
 }
 public static void start(ConversionMap conversionMap, Scanner userInput) throws MalformedLineException, IOException, NumberFormatException, NoSuchUnitException, NoPathBetweenUnits {
 // Convert
 System.out.println("Enter the first unit");
 String unit1 = userInput.next(unitRegex);
 System.out.println("Enter the amount of the first unit");
 double unit1Amount = userInput.nextDouble();
 System.out.println("Enter the unit to convert to");
 String unit2 = userInput.next(unitRegex);
 System.out.println(conversionMap.convert(unit1Amount, unit1, unit2));
 }
 public static ConversionMap readConversionMapFromFile(final String filename) throws FileNotFoundException, IOException, NumberFormatException, MalformedLineException {
 try(final BufferedReader inputReader = new BufferedReader(new InputStreamReader(new FileInputStream(filename)))) {
 return readConversionMapFromBufferedReader(inputReader);
 }
 }
 public static ConversionMap readConversionMapFromBufferedReader(BufferedReader inputReader) throws NumberFormatException, MalformedLineException, IOException {
 final ConversionMap conversionMap = new ConversionMap();
 // Parse every line
 for (String line = inputReader.readLine(); line != null; line = inputReader.readLine()) {
 parseLine(conversionMap, line);
 }
 return conversionMap;
 }
 private static void parseLine(ConversionMap conversionMap, String line) throws MalformedLineException, NumberFormatException {
 // Remove spaces
 line = line.replaceAll(" ", "");
 Matcher matcher = tokenPattern.matcher(line);
 // Make sure that the line matches the acceptable format
 if (!matcher.matches()) {
 throw new MalformedLineException("The line " + line + " does not conform to the regex.");
 }
 // Parse the numbers before the units
 double number1 = Double.parseDouble(matcher.group(1)),
 number2 = Double.parseDouble(matcher.group(3));
 // The two units
 String item1 = matcher.group(2),
 item2 = matcher.group(4);
 // Add a conversion relationship between these two
 conversionMap.addConversion(item1, number1, item2, number2);
 }
}

The search for a conversion path can be eliminated by using union-find. The gist is to only maintain conversions between a 'canonical' member of each set and the peripheral members. For example, the 'canonical' member of length conversions might be meter, and you only need to keep conversions to and from meters. Any other length can then be converted by a two-step path going through meters.

Wikipedia has a good article on union-find and disjoint sets.

• rolfl: Interesting that you recommend BigFraction and not the native BigDecimal in Java. That is an omission, I feel.
• Sam: @rolfl The reason BigDecimal is not appropriate is that it forces you to round every time you do a division. BigFraction can retain exact results through division.

Answer 2

Split ConversionsMain to muliple Classes

Right now, your ConversionsMain class gathers user input, reads conversion rules from a file, validates these rules, and of course starts your program. I would create separate classes for all these functionalities.

Right now, it's not all that bad, but if your program gets extended, a class like this can grow quite big very fast.

Conversion class

Your Conversion isn't really a conversion (of one unit to another). I would probably call it Unit instead.

And then I would create a Conversion class which holds two Units (currently this is a Set in your code). This class could also handle the actual conversion from one unit to another.

Encapsulation

units in ConversionPath should better be private and accessed by a getter.

getConversionPath(String unit1, String unit2, Map<String, ConversionPath> visited) should also be private, and probably the other getConversionPath method as well.

Getting User Input

You are generally handling exceptions very well, but when gathering user input, this can happen: java.util.InputMismatchException. It would be best to catch this and inform the user of wrong input.

It would also be helpful if the prompt gave an example of correct input.

You could also add a hasUnit method to your ConversionMap class, that way, you could give feedback about a non-existing unit as soon as a user enters it.

Reading Rules File

Right now, if a rule doesn't match the pattern, you throw an exception and the program quits. I would prefer it if malformed lines were skipped, so that your program still works even if one line is wrong.

Instead of throwing an exception, you could log the error in a file.

Naming

• as mentioned above, Conversion is not really a conversion and should probably be renamed.
• by convention, static final variable names should be all uppercase (so numberRegex should be NUMBER_REGEX.
• Exceptions should have Exception in their name. You forgot this for NoPathBetweenUnits (which I would rename to something like UnknownConversionException).
• pathLength can just be length, because the class is already named ConversionPath.

Comments

I generally really like your comments, so just some minor points:

• ConversionMap is missing a class comment.
• The path is immutable: this does not seem to be true, as units is public.
• If unit1 or unit2 exist but there is no way to convert between them: the or should be an and.
• some comments are not really necessary (eg // Loop through each unit in the path, // Get the shortest path, // Loop through each conversion, etc).

Misc

• use the @Override annotation (eg for toString, equals, etc).
• consistency: your Conversion has a getter for unitName, but not for unitAmount (this is accessed directly).
• your Conversion.equals method does not check if o is null before accessing it.