HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Yet Another Java GUI Calculator

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
javayetanotherguicalculator

Problem

In the spirit of April 2015 Community Challenge, I have created two Java-based calculators with the following codebase layout:

  • An interface Calculeightor for describing how values can be appended into a stream and then reduced to a single result given an operator.



  • An enum Operator to represent the operators.



  • The GUI implementation CalculeightorGUI.



  • The CLI implementation CalculeightorCLI.



  • A unit test for the CLI implementation CalculeightorCommandLineInterfaceTest.



The name uses eight as a pun, since this is written with Java 8 features in mind. Also, the unit test spells out CLI in full because appending Test to that creates another word that some may find inappropriate...

Without further ado, what follows are the code for Calculeightor and CalculeightorGUI. For the review of the other code, please look at the other review.

General feedback I am looking for:

  • Any places where I can simplify the logic?



  • Are Javadocs clear and concise enough?



  • Any major faults?



Specific feedback I am looking for It's Bounty Time:

  • I intended to keep the Calculeightor interface generic enough so that I can potentially cater for other classes extending Number, but since I have nailed down my Operator enum to support only Doubles, I have lost this flexibility. Would I be (marginally?) better off, for the purposes of this interface and the implementations, dropping the typed parameter and go with Double and Operator in it?



  • The reason behind the private enum State is to consolidate the toggling of buttons into a series of steps that a developer can tell at a glance. As such, places that require a 'refresh of the GUI simply calls updateState(), instead of scattering the toggling logic around. Is this an OK approach, or are there alternatives to this that I should be aware of?



  • Is there a simpler way of creating key bindings inside the GUI implementation?



Calculeightor

```
/**
* A lightweight interface for describing

Solution

For starters, my main() method should use SwingUtilities.invokeLater():

public static void main(String[] args) {
    SwingUtilities.invokeLater(() -> { new CalculeightorGUI(); });
}


I should also use my State enum to handle the display of inputs, for example I can have the following two static helper methods in the enum:

private static void display(CalculeightorGUI instance, Double value) {
    instance.label.setText(Integer.valueOf(value.intValue()).toString());
}

private static void display(CalculeightorGUI instance, String text) {
    instance.label.setText(text);
}


And the code for FIRST_OPERAND, OPERATOR and SECOND_OPERAND can be modified as such:

FIRST_OPERAND {
    @Override
    void set(CalculeightorGUI instance) {
        toggle(instance.numbers.stream(), instance.operators.stream());
        List inputs = instance.inputs;
        display(instance, inputs.get(inputs.size() - 1));
    }
},
OPERATOR {
    @Override
    void set(CalculeightorGUI instance) {
        toggle(instance.numbers.stream(), instance.operators.stream());
        display(instance, instance.operator.toString());
    }
},
SECOND_OPERAND {
    @Override
    void set(CalculeightorGUI instance) {
        toggle(instance.numbers.stream(), Stream.of(instance.equals));
        List inputs = instance.inputs;
        display(instance, inputs.get(inputs.size() - 1));
    }
}


This simplifies (slightly) my ActionListeners for the numbers and operators buttons:

private JPanel numbersPanel() {
    return panelOf(IntStream.rangeClosed(0, 9).boxed(), numbers, event -> {
        appendValue(Double.valueOf((((JButton) event.getSource()).getText())));
        updateState();
    });
}

private JPanel operatorsPanel() {
    return panelOf(Stream.of(Operator.values()), operators, event -> {
        setOperator(Operator.of(((JButton) event.getSource()).getText()));
        updateState();
    });
}


edit

Actually, why stop at the operands and operator when I can do the same thing for the equals button too?

I figured I can also create a Double result field in my class:

private final List inputs = new ArrayList<>(2);
private Double result = null; // to store the result
private State state = null;
private Operator operator;


Then my equalsButton() can be simplified as such:

private JButton equalsButton() {
    JButton button = createButton("=", event -> {
        result = inputs.stream().reduce(operator).get();
    });
    return button;
}


The Operator.EQUALS implementation then becomes:

EQUALS {
    @Override
    void set(CalculeightorGUI instance) {
        toggle(instance.numbers.stream(), Stream.of(instance.equals));
        instance.label.setText(instance.display(instance.result));
        instance.label.setForeground(
                instance.result.isInfinite() ? Color.RED : Color.BLACK);
        instance.inputs.clear();
    }
}


The equalsButton() changes above does not call the updateState(), ditto for the ActionListeners specified in numbersPanel() and operatorsPanel(), because I can move that to my createButton() code to 'compose' invoking the listener argument with calling updateState() too. This means I have to un-static the method createButton() itself.

private JButton createButton(Object input, ActionListener listener) {
    String text = input.toString();
    JButton button = new JButton(text);
    button.addActionListener(event -> {
        listener.actionPerformed(event); updateState(); });
    button.getInputMap(JComponent.WHEN_IN_FOCUSED_WINDOW).put(
            KeyStroke.getKeyStroke(text.charAt(0)), text);
    button.getActionMap().put(text, getAction(button));
    return button;
}

Code Snippets

public static void main(String[] args) {
    SwingUtilities.invokeLater(() -> { new CalculeightorGUI(); });
}
private static void display(CalculeightorGUI instance, Double value) {
    instance.label.setText(Integer.valueOf(value.intValue()).toString());
}

private static void display(CalculeightorGUI instance, String text) {
    instance.label.setText(text);
}
FIRST_OPERAND {
    @Override
    void set(CalculeightorGUI instance) {
        toggle(instance.numbers.stream(), instance.operators.stream());
        List<Double> inputs = instance.inputs;
        display(instance, inputs.get(inputs.size() - 1));
    }
},
OPERATOR {
    @Override
    void set(CalculeightorGUI instance) {
        toggle(instance.numbers.stream(), instance.operators.stream());
        display(instance, instance.operator.toString());
    }
},
SECOND_OPERAND {
    @Override
    void set(CalculeightorGUI instance) {
        toggle(instance.numbers.stream(), Stream.of(instance.equals));
        List<Double> inputs = instance.inputs;
        display(instance, inputs.get(inputs.size() - 1));
    }
}
private JPanel numbersPanel() {
    return panelOf(IntStream.rangeClosed(0, 9).boxed(), numbers, event -> {
        appendValue(Double.valueOf((((JButton) event.getSource()).getText())));
        updateState();
    });
}

private JPanel operatorsPanel() {
    return panelOf(Stream.of(Operator.values()), operators, event -> {
        setOperator(Operator.of(((JButton) event.getSource()).getText()));
        updateState();
    });
}
private final List<Double> inputs = new ArrayList<>(2);
private Double result = null; // to store the result
private State state = null;
private Operator operator;

Context

StackExchange Code Review Q#88005, answer score: 5

Revisions (0)

No revisions yet.