5
\$\begingroup\$

I have a Swing application that converts lengths. Inches to centimeter, feet to yard, etc. I’m wondering if I could make the super large HashMap a little smaller, or more efficient. Any other suggestions are also appreciated.

import java.awt.*;
import java.awt.event.*;
import java.util.HashMap;
import javax.swing.*;
public class LengthConverter{
    private JFrame frame;
    private JLabel welcomeLabel;
    private JPanel fromPanel;
    private JLabel centerLabel;
    private JTextField fromField;
    private JComboBox<String> fromBox;
    private JPanel toPanel;
    private JTextField toField;
    private JComboBox<String> toBox;
    private JButton submitButton;
    private JLabel noteLabel;
    private JPanel bottomPanel;
    private final String[] lengthTypes = {"Inches", "Centimeters", "Feet", "Yards", "Meters", "Millimeters"};
    private final HashMap<String, HashMap<String, Double>> conversions = new HashMap<>();
    public LengthConverter() {
        conversions.put("Inches", new HashMap<>());

        conversions.get("Inches").put("Inches", 1.0);
        conversions.get("Inches").put("Centimeters", 2.54);
        conversions.get("Inches").put("Feet", 0.0833333);
        conversions.get("Inches").put("Yards", 0.0277778);
        conversions.get("Inches").put("Meters", 0.0254);
        conversions.get("Inches").put("Millimeters", 25.4);

        conversions.put("Centimeters", new HashMap<>());

        conversions.get("Centimeters").put("Inches", 0.393701);
        conversions.get("Centimeters").put("Centimeters", 1.0);
        conversions.get("Centimeters").put("Feet", 0.0328084);
        conversions.get("Centimeters").put("Yards", 0.0109361);
        conversions.get("Centimeters").put("Meters", 0.01);
        conversions.get("Centimeters").put("Millimeters", 10.0);

        conversions.put("Feet", new HashMap<>());

        conversions.get("Feet").put("Inches", 12.0);
        conversions.get("Feet").put("Centimeters", 30.48);
        conversions.get("Feet").put("Feet", 1.0);
        conversions.get("Feet").put("Yards", 0.3333333);
        conversions.get("Feet").put("Meters", 0.3048);
        conversions.get("Feet").put("Millimeters", 304.8);

        conversions.put("Yards", new HashMap<>());

        conversions.get("Yards").put("Inches", 36.0);
        conversions.get("Yards").put("Centimeters", 91.44);
        conversions.get("Yards").put("Feet", 3.0);
        conversions.get("Yards").put("Yards", 1.0);
        conversions.get("Yards").put("Meters", 0.9144);
        conversions.get("Yards").put("Millimeters", 914.4);

        conversions.put("Meters", new HashMap<>());

        conversions.get("Meters").put("Inches", 39.3701);
        conversions.get("Meters").put("Centimeters", 100.0);
        conversions.get("Meters").put("Feet", 3.28084);
        conversions.get("Meters").put("Yards", 1.09361);
        conversions.get("Meters").put("Meters", 1.0);
        conversions.get("Meters").put("Millimeters", 1000.0);

        conversions.put("Millimeters", new HashMap<>());

        conversions.get("Millimeters").put("Inches", 0.0393701);
        conversions.get("Millimeters").put("Centimeters", 0.1);
        conversions.get("Millimeters").put("Feet", 0.00328084);
        conversions.get("Millimeters").put("Yards", 0.00109361);
        conversions.get("Millimeters").put("Meters", 0.001);
        conversions.get("Millimeters").put("Millimeters", 1.0);
    }
    public static void main(String[] args) {
        new LengthConverter().go();
    }
    public void go() {
        frame = new JFrame("Length Conversions");
        fromPanel = new JPanel();
        centerLabel = new JLabel("Is...");
        fromField = new JTextField(10);
        fromBox = new JComboBox<>(lengthTypes);
        toPanel = new JPanel();
        toBox = new JComboBox<>(lengthTypes);
        toField = new JTextField(10);
        toField.setEnabled(false);
        welcomeLabel = new JLabel("Welcome to the length converter!");
        submitButton = new JButton("Convert!");
        noteLabel = new JLabel("Note: Rounds to fit the text field.");
        bottomPanel = new JPanel();
        submitButton.addActionListener(new SubmitListener());

        fromPanel.setLayout(new BoxLayout(fromPanel, BoxLayout.X_AXIS));
        fromPanel.add(fromField);
        fromPanel.add(fromBox);

        toPanel.setLayout(new BoxLayout(toPanel, BoxLayout.X_AXIS));
        toPanel.add(toField);
        toPanel.add(toBox);

        bottomPanel.setLayout(new BoxLayout(bottomPanel, BoxLayout.X_AXIS));
        bottomPanel.add(noteLabel);
        bottomPanel.add(submitButton);

        Font labelFont = new Font("Arial", Font.BOLD, 24);
        welcomeLabel.setFont(labelFont);

        frame.setLayout(new BorderLayout());
        Container window = frame.getContentPane();

        window.add(BorderLayout.NORTH, welcomeLabel);
        window.add(BorderLayout.WEST, fromPanel);
        window.add(BorderLayout.EAST, toPanel);
        window.add(BorderLayout.CENTER, centerLabel);
        window.add(BorderLayout.SOUTH, bottomPanel);

        frame.pack();
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.setVisible(true);
    }
    class SubmitListener implements ActionListener {

        @Override
        public void actionPerformed(ActionEvent e) {
            Double numToConvert = Double.parseDouble(fromField.getText());
            Double conversionRate = conversions.get(fromBox.getSelectedItem()).get(toBox.getSelectedItem());
            String result = Double.toString(numToConvert * conversionRate);
            int length = result.length();
            if (length > 4) {
                Dimension d = frame.getSize();
                frame.setSize(length + d.width - 16, d.height);
                toField.setColumns(length);
            }
            else {
                toField.setColumns(4);
                frame.pack();
            }
            toField.setText(result);
            frame.revalidate();
        }

    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ 36 entries in your conversions where you really only have 7 items. Don't repeat yourself! \$\endgroup\$ – Loren Pechtel Apr 22 '15 at 1:06
6
\$\begingroup\$

Instead of a nested hash map containing the conversion factor from every unit to every other unit, it is sufficient to have a mapping from each unit to a single common unit, e.g. meter:

HashMap<String, Double> conversions = new HashMap<>();

conversions.put("Centimeters", 0.01);
conversions.put("Inches", 0.0254);
// ...

Then the conversion factor between two units can simply be calculated as

Double conversionRate = conversions.get(fromUnit) / conversions.get(toUnit);

Actually you don't need a hash map at all, you could store the information in two parallel arrays:

private final String[] lengthTypes = { "Inches", "Centimeters", "Feet", "Yards", "Meters", "Millimeters"};
private final double[] conversionFactors = { 0.054, 0.01, 0.3048, 0.9144, 1.0, 0.001 };

Then

double conversionRate = conversionFactors[fromBox.getSelectedIndex()] / conversionFactors[toBox.getSelectedIndex()];

Alternatively, define a

class Unit {
    String name;
    double conversionFactor;

    Unit(String name, double conversionFactor) {
        this.name = name;
        this.conversionFactor = conversionFactor;
    }
}

and the list of available units with

Unit[] units = {
    new Unit("Centimeters", 0.01),
    new Unit("Inches", 0.0254)
    // ...
};

Then the unit name and corresponding factor is defined together, and it is less error-prone if units are reordered or renamed. The combo boxes would then be populated with

for (Unit unit : units) {
    fromBox.addItem(unit.name);
    toBox.addItem(unit.name);
}

and the conversion factor between two units is

double conversionRate = units[fromBox.getSelectedIndex()].conversionFactor / units[toBox.getSelectedIndex()].conversionFactor;
\$\endgroup\$
  • 1
    \$\begingroup\$ The idea of using a single common unit has the added advantage that conversions are consistent, so that converting back and forth (cm->inch->cm) will always give the original answer (up to the floating point precision). \$\endgroup\$ – Rob Apr 22 '15 at 7:57
2
\$\begingroup\$

What does a LengthConverter do? Does it show buttons, react to clicks? Or convert lengths?

This is clear case of a violation of the Separation of concerns principle. Simply extract the conversion to a separate class.

Actually, Martin R.'s Unit does it already. You could also use an enum (the annotations come from Lombok, but it should be obvious what they do).

@RequiredArgsConstructor @Getter
public enum Unit {
    METER(1.0),
    CENTIMETER(0.01),
    INCH(0.0254),
    FURLONG(201.168),
    ...
    ;

    double convertFrom(double amount, Unit unit) {
        return amount * unit.meters / this.meters;
    }

    private final double meters;
}

with an expression like

Unit.INCH.convertFrom(2, Unit.LIGHT_YEAR);

telling you how many inches are 2 light years. No map needed.

The obvious disadvantage of enums is that they can't be extended, but for the author this is no problem (and the set of useful units is rather limited, anyway).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.