Simple Java calculator using Swing

Question

This is a Java calculator I made for my programming class. I know there must be vast room for improvement.

Please bear in mind the fact that this only accepts an integer, then an operator, then another integer, and finally the = button to function correctly. Also please bear in mind the fact that it prints the result using System.out.print().

import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import javax.swing.JButton;
import javax.swing.JFrame;

public class JavaCalculator
{
    public static boolean isA, isB;
    public static int a, b, answer;
    public static String operator;

    public static void main (String[] args)
    {
        JButton button_one = new JButton ("1");
        button_one.setBounds (6, 86, 100, 40);
        button_one.addActionListener (new ActionListener()
        {
            public void actionPerformed (ActionEvent event)
            {
                if (!isA && !isB)
                {
                    a = 1;
                    isA = true;
                    System.out.print (a);
                }
                else if (isA && !isB)
                {
                    b = 1;
                    isB  = true;
                    System.out.print (" " + b);
                }
            }
        });

        JButton button_two = new JButton ("2");
        button_two.setBounds (106, 86, 100, 40);
        button_two.addActionListener (new ActionListener()
        {
            public void actionPerformed (ActionEvent event)
            {
                if (!isA && !isB)
                {
                    a = 2;
                    isA = true;
                }
                else if (isA && !isB)
                {
                    b = 2;
                    isB  = true;
                }
            }
        });

        JButton button_three = new JButton ("3");
        button_three.setBounds (206, 86, 100, 40);
        button_three.addActionListener (new ActionListener()
        {
            public void actionPerformed (ActionEvent event)
            {
                if (!isA && !isB)
                {
                    a = 3;
                    isA = true;
                }
                else if (isA && !isB)
                {
                    b = 3;
                    isB  = true;
                }
            }
        });

        JButton button_four = new JButton ("4");
        button_four.setBounds (6, 126, 100, 40);
        button_four.addActionListener (new ActionListener()
        {
            public void actionPerformed (ActionEvent event)
            {
                if (!isA && !isB)
                {
                    a = 4;
                    isA = true;
                }
                else if (isA && !isB)
                {
                    b = 4;
                    isB  = true;
                }
            }
        });

        JButton button_five = new JButton ("5");
        button_five.setBounds (106, 126, 100, 40);
        button_five.addActionListener (new ActionListener()
        {
            public void actionPerformed (ActionEvent event)
            {
                if (!isA && !isB)
                {
                    a = 5;
                    isA = true;
                }
                else if (isA && !isB)
                {
                    b = 5;
                    isB  = true;
                }
            }
        });

        JButton button_six = new JButton ("6");
        button_six.setBounds (206, 126, 100, 40);
        button_six.addActionListener (new ActionListener()
        {
            public void actionPerformed (ActionEvent event)
            {
                if (!isA && !isB)
                {
                    a = 6;
                    isA = true;
                }
                else if (isA && !isB)
                {
                    b = 6;
                    isB  = true;
                }
            }
        });

        JButton button_seven = new JButton ("7");
        button_seven.setBounds (6, 168, 100, 40);
        button_seven.addActionListener (new ActionListener()
        {
            public void actionPerformed (ActionEvent event)
            {
                if (!isA && !isB)
                {
                    a = 7;
                    isA = true;
                }
                else if (isA && !isB)
                {
                    b = 7;
                    isB  = true;
                }
            }
        });

        JButton button_eight = new JButton ("8");
        button_eight.setBounds (106, 168, 100, 40);
        button_eight.addActionListener (new ActionListener()
        {
            public void actionPerformed (ActionEvent event)
            {
                if (!isA && !isB)
                {
                    a = 8;
                    isA = true;
                }
                else if (isA && !isB)
                {
                    b = 8;
                    isB  = true;
                }
            }
        });

        JButton button_nine = new JButton ("9");
        button_nine.setBounds (206, 168, 100, 40);
        button_nine.addActionListener (new ActionListener()
        {
            public void actionPerformed (ActionEvent event)
            {
                if (!isA && !isB)
                {
                    a = 9;
                    isA = true;
                }
                else if (isA && !isB)
                {
                    b = 9;
                    isB  = true;
                }
            }
        });

        JButton button_zero = new JButton ("0");
        button_zero.setBounds (106, 208, 100, 40);
        button_zero.addActionListener (new ActionListener()
        {
            public void actionPerformed (ActionEvent event)
            {
                if (!isA && !isB)
                {
                    a = 0;
                    isA = true;
                }
                else if (isA && !isB)
                {
                    b = 0;
                    isB  = true;
                }
            }
        });

        JButton button_equals = new JButton ("=");
        button_equals.setBounds (312, 168, 58, 80);
        button_equals.addActionListener (new ActionListener()
        {
            public void actionPerformed (ActionEvent event)
            {
                if (operator.contains ("-"))
                {
                    answer = a - b;
                    System.out.print (answer);
                }
                else if (operator.contains ("+"))
                {
                    answer = a + b;
                    System.out.print (answer);
                }
                else if (operator.contains ("/"))
                {
                    answer = a / b;
                    System.out.print (answer);
                }
                else if (operator.contains ("*"))
                {
                    answer = a * b;
                    System.out.print (answer);
                }
            }
        });

        JButton button_minus = new JButton ("-");
        button_minus.setBounds (312, 126, 58, 40);
        button_minus.addActionListener (new ActionListener()
        {
            public void actionPerformed (ActionEvent event)
            {
                operator = "-";
            }
        });

        JButton button_plus = new JButton ("+");
        button_plus.setBounds (312, 86, 58, 40);
        button_plus.addActionListener (new ActionListener()
        {
            public void actionPerformed (ActionEvent event)
            {
                operator = "+";
            }
        });

        JButton button_divide = new JButton ("/");
        button_divide.setBounds (312, 46, 58, 40);
        button_divide.addActionListener (new ActionListener()
        {
            public void actionPerformed (ActionEvent event)
            {
                operator = "/";
            }
        });

        JButton button_times = new JButton ("*");
        button_times.setBounds (312, 6, 58, 40);
        button_times.addActionListener (new ActionListener()
        {
            public void actionPerformed (ActionEvent event)
            {
                operator = "*";
            }
        });

        JFrame frame = new JFrame ("Java Calculator");
        frame.setDefaultCloseOperation (JFrame.EXIT_ON_CLOSE);
        frame.setSize (380, 274);
        frame.setResizable (false);
        frame.setLayout (null);
        frame.add (button_one);
        frame.add (button_two);
        frame.add (button_three);
        frame.add (button_four);
        frame.add (button_five);
        frame.add (button_six);
        frame.add (button_seven);
        frame.add (button_eight);
        frame.add (button_nine);
        frame.add (button_zero);
        frame.add (button_equals);
        frame.add (button_minus);
        frame.add (button_plus);
        frame.add (button_divide);
        frame.add (button_times);
        frame.setVisible (true);
    }
}

Welcome to CodeReview, noobinator. I hope you get some fine answers! — Legato, Apr 23 '15 at 20:35
Hi! I've added the community-challenge tag, as it just so happens that we're having a community challenge about implementing simple calculators! Welcome to Code Review! — Mathieu Guindon, Apr 23 '15 at 20:37
It looks like all the button generating code can be put inside a loop, but I'm not familiar with Java I will leave it to someone else. — Caridorc, Apr 23 '15 at 20:46
I understand that it outputs to the console, but is that a requirement? — Legato, Apr 23 '15 at 22:23

Legato · Accepted Answer · 2015-05-08 23:21:11Z

You don't need so many anonymous handlers, would be better to have one just track the source of the event and follow the logic you want. You haven't yet answered whether printing to the console is a requirement... so I'll hold off on expanding this until that's answered.

Also, you can streamline the way you create the buttons with an array, consider using a panel with a layout manager to house the numbers.

JPanel numberPanel = new JPanel(new GridLayout(0, 3));
JButton[] numbers = new JButton[10];

numbers[0] = new JButton("0"); // while you can do this
// Better to use a loop!
for (int i = 1; i < numbers.length; i++) {
        numbers[i] = new JButton(Integer.toString(i));
        numberPanel.add(numbers[i]);
}
// Of course we don't forget the 0
numberPanel.add(numbers[0]);

All you need to do at the end is frame.add(numberPanel)

You can do the same thing for the operators, and may use a border to manage distance between them, adding in Border layout with your frame to control overall positioning.

JPanel operatorPanel = new JPanel(new GridLayout(0, 1));
operatorPanel.setBorder(BorderFactory.createEmptyBorder(0, 5, 0, 0));
JButton[] operators = new JButton[5];

Right now, I have an implementation that loops through an enum set with the operators and uses a lambda expression within to compute, but you've yet to answer my question about whether or not this has to be implemented on the console, so I'm not sure if that's within scope for your purposes. Though, I do think it makes more sense to also have your output be a JLabel that has its own space.For now, you can have this loop through a string of the symbols{"+", "-", "*", "/", "="} and add them the same way that's already been expressed.

So at the end, the three things you'd add:

frame.add(result, BorderLayout.NORTH);
frame.add(numberPanel, BorderLayout.CENTER);
frame.add(operatorPanel, BorderLayout.EAST);

Caridorc · Accepted Answer · 2015-04-24 12:15:43Z

Let me start from the simplest improvement:

Use a foreach loop:

frame.add (button_one);
frame.add (button_two);
frame.add (button_three);
frame.add (button_four);
frame.add (button_five);
frame.add (button_six);
frame.add (button_seven);
frame.add (button_eight);
frame.add (button_nine);
frame.add (button_zero);
frame.add (button_equals);
frame.add (button_minus);
frame.add (button_plus);
frame.add (button_divide);
frame.add (button_times);

should become:

for (Button : new JButton[]{button_one, button_two, ... button_times}) {
    frame.add (button);
}

Jamal · Accepted Answer · 2015-04-23 21:01:33Z

You appear to repeat this similar piece of code:

if (!isA && !isB)
{
    a = 1;
    isA = true;
    System.out.print (a);
}
else if (isA && !isB)
{
    b = 1;
    isB  = true;
    System.out.print (" " + b);
}

Consider putting this into a separate method, so that you can just define this once and call the function in those places. You may just need to pass in the value that will be set to a and b.

Robert Snyder · Accepted Answer · 2015-04-24 03:23:09Z

Since you mentioned a programming class, I find that most students would greatly benefit from learning about Unit Tests. It isn't often taught, but I believe it should be. One main reason is that it forces you to be make code that is able to have automated tests. Automated tests are awesome because they are fast, and give you feedback if you made a mistake or not. There are a few prominent approaches to writing tests and the purpose is to not make you use a specific one. I can say that I prefer to write my tests first then write my production code. I do it this way because, for myself, I name methods a bit more with english in mind than programming. This takes a small amount of brain strain out of programming. (FYI the paradigm I mention about writing tests first is TDD, but like I said there are a few other ways of doing it they all have their advantages and disadvantages).

So although it may seem trivial to write a test for a calculator one thing that it could possibly do is make choosing where you show your output even more trivial. (I've read in a few of the answers people mentioning where they would put the output.) You mentioned how you want your code to flow so I will focus writing a test that matches what you wanted. (Note the final code is almost complete, just need to finish implementing the operators.)

IntegerCalculatorNGTest.java

import static org.testng.Assert.*;
import org.testng.annotations.Test;

public class IntegerCalculatorNGTest {

    public IntegerCalculatorNGTest() {
    }

    @Test
    public void testWhenNoNumbersAreEnteredErrorMessageIsReturned() {
        IntegerCalculator calculator = new IntegerCalculator();
        String result = calculator.getResult();

        assertEquals(result, "No Numbers Are Present");
    }

    @Test
    public void testWhenOnlyNumberAIsEnteredErrorMessageIsReturned() {
        IntegerCalculator calculator = new IntegerCalculator();
        calculator.addNumber(1);
        String result = calculator.getResult();

        assertEquals(result, "Atleast 2 Numbers Required");
    }

    @Test
    public void testWhenNoOperatorIsEnteredAfterAddingOneNumberExceptionIsThrown() {
        IntegerCalculator calculator = new IntegerCalculator();
        calculator.addNumber(1);
        try{
            calculator.addNumber(2);
        }
        catch(IllegalStateException ex){
            assertEquals(ex.getMessage(), "Operator Must Be Set before Adding Second Number");
            return;
        }
        fail("Method should have thrown an exception");
    }

    @Test
    public void testWhenAThirdNumberIsAddedExceptionIsThrown() {
        IntegerCalculator calculator = new IntegerCalculator();
        calculator.addNumber(1);
        calculator.setOperator("+");
        calculator.addNumber(2);
        try{
            calculator.addNumber(2);
        }
        catch(IllegalStateException ex){
            assertEquals(ex.getMessage(), "For now, only two numbers can be used");
            return;
        }
        fail("Method should have thrown an exception");
    }

    @Test
    public void testWhenNumberThenOperatorThenNumberIsEnteredValidResultIsReturned() {
        IntegerCalculator calculator = new IntegerCalculator();
        calculator.addNumber(1);
        calculator.setOperator("+");
        calculator.addNumber(2);

        String result = calculator.getResult();

        assertEquals(result, "3");
    }
}

ok, so now in roughly 0.015 seconds I can see if any changes I make to IntegerCalculator broke my logic. I also know exactly how to use it and can refer to my tests as a form of documentation. Ok, If you pasted that above test in, then you'll know that IntegerCalculator does not exist. Here it is (well, most of it)

IntegerCalculator.java

import java.util.ArrayList;
import java.util.List;

/**
 * @author snyder
 */
public class IntegerCalculator {
    private final List<Integer> _numbers;
    private String _operator;

    public IntegerCalculator() {
        this._numbers = new ArrayList<>();
        this._operator = "";
    }


    String getResult() {
        if(_numbers.isEmpty())
            return "No Numbers Are Present";
        else if (_numbers.size() < 2)
            return "Atleast 2 Numbers Required";
        int result = 0;
        switch(_operator){
            case "+":
                result = _numbers.get(0) + _numbers.get(1);
                break;
        }

        return Integer.toString(result);
    }

    void addNumber(int i) {
        if(!_numbers.isEmpty() && _operator.isEmpty())
            throw new IllegalStateException("Operator Must Be Set before Adding Second Number");

        if(_numbers.size() == 2)
            throw new IllegalStateException("For now, only two numbers can be used");

        _numbers.add(i);
    }

    void setOperator(String operator) {
        this._operator = operator;
    }
}

ok, so now how do I use it? if you listened to h.j.k and use a common actionListener for all your number buttons, and another for your operator buttons then it is super simple.

here are the important parts that i did to make it work (it's not super pretty)

private static IntegerCalculator _calculator = new IntegerCalculator();

private static JButton createDefaultNumberButton(int index) {
    JButton numberButton = new JButton(Integer.toString(index));
    numberButton.setPreferredSize(new Dimension(50, 25));
    numberButton.addActionListener(event -> {
        try {
            _calculator.addNumber(Integer.valueOf(((JButton) event.getSource()).getText()));
        } catch (IllegalStateException ex) {
            System.out.println(ex.getMessage());
        }
    });
    return numberButton;
}

public static void main(String[] args) {
    JPanel numberPanel = new JPanel(new GridLayout(0, 3));

    for (int i = 1; i < 10; i++) {
        numberPanel.add(createDefaultNumberButton(i));
    }

    numberPanel.add(createDefaultNumberButton(0));
    numberPanel.setBounds(5, 5, 150, 200);

    JButton button_equals = new JButton("=");
    button_equals.setBounds(312, 168, 58, 80);
    button_equals.addActionListener((ActionEvent event) -> {
        System.out.print(_calculator.getResult());
    });

    JButton button_plus = new JButton("+");
    button_plus.setBounds(312, 86, 58, 40);
    button_plus.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent event) {
            _calculator.setOperator("+");
        }
    });

the IntegerCalculator class is still a little bit from being complete, and the GUI itself as it stands right now can't add numbers larger than 1 number, but you get the jist of it.

Community · Accepted Answer · 2017-04-13 12:40:41Z

Layout

As mentioned in @Legato's answer, you can use a panel and a layout manager to organize the layout, so that you are not constrained to alignment by integers (pixels I believe?).

Validating flow of inputs

You should be able to do something smarter here... how about considering disabling the operators' buttons before the first input, then disable the numbers' buttons before an operator is selected, and then finally enable only the equals button after the second number? Using a Collection to reference your two sets of buttons will simplify the code to perform this toggling significantly. :)

Storing input values

You shouldn't need a pair of boolean values on top of your two fields to track whether you have one or two numbers. A single List will suffice, and you can check if the size() is 0 or 1.

Implementing the `ActionListener`

If you happen to be using Java 8, your call to an ActionListener can be simplified greatly too:

numberButton.addActionListener(event -> {
    // assuming input validation is done, 
    // e.g. the disabling/enabling feature mentioned above
    inputs.add(Integer.valueOf(((JButton) event.getSource()).getText()));
    // now disable all numeric buttons, 
    // and enable either the operators or equals buttons
});

Stack Exchange Network

current community

your communities

more stack exchange communities

Simple Java calculator using Swing

5 Answers 5

Layout

Validating flow of inputs

Storing input values

Implementing the `ActionListener`

Your Answer

Hot Network Questions

Simple Java calculator using Swing

5 Answers 5

Layout

Validating flow of inputs

Storing input values

Implementing the ActionListener

Your Answer

Sign up or log in

Post as a guest

Related

Hot Network Questions

Implementing the `ActionListener`