5
\$\begingroup\$

I know this might be a relatively large amount of code, but I'm just looking for any improvement, big or small, on my coding as I can.

As part of my self-study in programming, I decided to make a calculator in Java Swing with MVC in mind. The calculator will be capable of handling lengthy sequences of different operations, instead of just 2 operands and 1 operator, and performs them in order according to the MDAS precedence rule.

Please take note that I'm trying to implement the original MVC proposed by the SmallTalk team (which implements the Observer pattern relationship between the Model and the View), as opposed to what everyone now is calling MVC, which is the Cocoa-style MVC (which implements a Mediator pattern with the Controller serving as Mediator between View and Controller).

This is a basic breakdown of how the program works:

All three main parts of MVC are instantiated within one class. The View is made up of buttons, all which share the same listener. The Total Display and the Computation Display both receive their data from the Model in the form of CalcDisplayData updates. The Controller defines the single listener that all buttons share, and this listener defines the logic that one would expect of a calculator. All of the button presses will be sending their respective values (in String form) through the assigned listener, towards the formation of a single currentInputString held in class Model.

When the "=" button is pressed, the currentInputString is put into a parsing algorithm, which chops it up into a list of tokens composed of operands, and operators.

Then an operation performing algorithm then proceeds to perform operations of the first kind, multiplication. It starts by finding the first * operator, and takes note of the operand that precedes and follows it in the list. It calculates the result of this operation,removes operands and operator in the list, and replaces these with the result.

It does this with all operations of this category, one after the other, until it can find no more. It then repeats the series with the next category of operations (in the MDAS manner).

After all operations are finished, the list should only consist of one token, which is the result.

The result is in float form though. This is compared to the same value, but cast into the int type. If they are the same, the int is then cast into a String and sends this as an update into the totalDisplay JTextArea in the View. Otherwise, the float value is what gets cast and sent.

Default Package

An initial class creates the Model, View, and Controller classes. It gives them the assigned references, and sets the View as an Observer of the Model.

Class Run

import javax.swing.SwingUtilities;

import Controller.Controller;
import Model.Model;
import View.View;

public class Run {

    public static void main(String[] args) {

        SwingUtilities.invokeLater(new Runnable() {

            public void run() {

                Model model = new Model();
                View view = new View(model);
                Controller controller = new Controller(model, view);

                model.addObserver(view);

            }
        });

    }

}

Package Model

Class Model

This is the main part of the Model.

package Model;

import java.util.LinkedList;
import java.util.Observable;

public class Model extends Observable {

    private String currentTotal;
    private String currentInputString;

    public Model() {
        currentTotal = "0";
        currentInputString = "";
    }

    public void computeString() {

        LinkedList<String> operationTokens = new StringParser(currentInputString).getTokens();

        MathOperationsList possibleOperations = new MathOperationsList();

        operationTokens = performMathInSequence(operationTokens, possibleOperations);

        boolean hasOnlyOneToken = (operationTokens.size() == 1);

        if (hasOnlyOneToken) {
            setCurrentTotal(operationTokens.get(0));
        } else {
            System.out.println("uhh.. something went wrong? LOL!");
        }

    }

    private LinkedList<String> performMathInSequence(LinkedList<String> operationTokens, MathOperationsList possibleOperations) {
        for (String operation : possibleOperations) {
            operationTokens = performOperations(operation, operationTokens);
        }
        return operationTokens;
    }

    private LinkedList<String> performOperations(String operation, LinkedList<String> tokens) {

        boolean isOperationCompleted = false;

        while (isOperationCompleted == false) {
            if (tokens.contains(operation)) {
                int operatorIndex = tokens.indexOf(operation);
                int firstOperandIndex = operatorIndex - 1;
                int secondOperandIndex = operatorIndex + 1;

                String firstOperand = tokens.get(operatorIndex - 1);
                String secondOperand = tokens.get(operatorIndex + 1);
                float computationResult;

                // perform the relevant operation
                switch (operation) {
                case "*":computationResult = Float.parseFloat(firstOperand)*Float.parseFloat(secondOperand);break;
                case "/":computationResult = Float.parseFloat(firstOperand)/Float.parseFloat(secondOperand);break;
                case "+":computationResult = Float.parseFloat(firstOperand)+Float.parseFloat(secondOperand);break;
                case "-":computationResult = Float.parseFloat(firstOperand)-Float.parseFloat(secondOperand);break;
                default:computationResult = (float) 69.69;
                    System.out.println("Cannot detect operation"); break;
                }

                // cast the operation back into a String
                String tokenizedComputation = Float.toString(computationResult);

                // remove all relevant tokens
                tokens.remove(secondOperandIndex);
                tokens.remove(operatorIndex);
                tokens.remove(firstOperandIndex);

                // place relevant token into relevant position
                tokens.add(firstOperandIndex, tokenizedComputation);

            } else {

                isOperationCompleted = true;
                return tokens;

            }

        }

        return tokens;
    }

    public void Clear() {
        currentTotal = "0";
        currentInputString = "";

        setChanged();

        CalcDisplayData update = new CalcDisplayData();
        update.setComputationText(currentInputString);
        update.setCurrentTotal(currentTotal);

        notifyObservers(update);

    }

    public void setComputationText(String newInputString) {
        currentInputString = newInputString;

        setChanged();

        CalcDisplayData update = new CalcDisplayData();
        update.setComputationText(newInputString);

        notifyObservers(update);

    }

    public void setCurrentTotal(String newTotal) {
        float floatTotal = Float.parseFloat(newTotal);
        int intTotal = (int) floatTotal;

        setCurrentTotalAsIntValueIfPossible(floatTotal, intTotal);

        setChanged();

        CalcDisplayData update = new CalcDisplayData();
        update.setCurrentTotal(currentTotal);

        notifyObservers(update);

    }

    private void setCurrentTotalAsIntValueIfPossible(float floatTotal, int intTotal) {
        if (floatTotal == intTotal) {
            currentTotal = Integer.toString(intTotal);
        } else {
            currentTotal = Float.toString(floatTotal);
        }
    }

}

Class StringParser

This is the algorithm for reading the computation text, and chopping it up into a list of tokens for further processing.

package Model;

import java.util.Arrays;
import java.util.LinkedList;
import java.util.List;

public class StringParser {

    static public final String WITH_DELIMITERS = "((?<=%1$s)|(?=%1$s))";
    LinkedList<String> answers;

    public StringParser(String string) {

        List<String> tokenList = extractTokens(string);
        answers = transformToLinkedList(tokenList);

    }

    public LinkedList<String> getTokens() {
        return answers;
    }

    private List<String> extractTokens(String string) {

        String[] tokens = string
                .split(String.format(WITH_DELIMITERS, "[*/+-]"));
        List<String> linkedTokens = Arrays.asList(tokens);

        return linkedTokens;
    }

    private LinkedList<String> transformToLinkedList(List<String> tokenList) {
        LinkedList<String> answers = new LinkedList<String>();
        answers.addAll(tokenList);

        return answers;
    }

}

Class CalcDisplayData

This class is an object encapsulating attribute values sent towards the View (which implements the Observer interface) from the Model when the Displays need to be updated.

package Model;

public class CalcDisplayData {

    String currentTotal;
    String currentInputString;

    public void setComputationText(String computationText) {
        currentInputString = computationText;
    }

    public void setCurrentTotal(String newTotal) {
        currentTotal = newTotal;
    }

    public String getCurrentTotal() {
        return currentTotal;
    }

    public String getCurrentInputString() {
        return currentInputString;
    }

}

Class MathOperationsList

Just an ArrayList that holds the four operations needed for the calculator to work.

package Model;

import java.util.ArrayList;

public class MathOperationsList extends ArrayList <String>{

    public MathOperationsList(){
        super();
        add("*");
        add("/");
        add("+");
        add("-");   
    }

}

Package View

Class View

This is the view which creates a series of buttons, places them all into an ArrayList, and uses an Iterator to place them one-by-one into the GridBagLayout.

package View;
import java.awt.Dimension;
import java.awt.Font;
import java.awt.GridBagConstraints;
import java.awt.GridBagLayout;
import java.awt.event.ActionListener;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.Observable;
import java.util.Observer;

import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JTextField;

import Model.CalcDisplayData;
import Model.Model;


public class View extends JFrame implements Observer {

    private JTextField totalDisplay;
    private JTextField computationDisplay;  
    private ArrayList <JButton> buttonList;

    public View(Model model) {

        super("MVC Calculator");
        setWindowPreferences();

        totalDisplay = new JTextField();
        configureTotalDisplay();

        computationDisplay = new JTextField();
        configureComputationDisplay();

        createButtonList();
        implementGridLayout();
    }

    private void configureComputationDisplay() {
        computationDisplay.setPreferredSize(new Dimension(600, 40));
        computationDisplay.setColumns(40);
        computationDisplay.setEditable(false);
        computationDisplay.setText("");
        computationDisplay.setHorizontalAlignment(JTextField.CENTER);
    }

    private void implementGridLayout() {
        GridBagConstraints gbc = new GridBagConstraints();
        setLayout(new GridBagLayout());     
        addComponentsToGrid(gbc);
    }

    private void addComponentsToGrid(GridBagConstraints gbc) {
        //text display//
        gbc.gridx = 0;
        gbc.gridy = 0;
        gbc.fill = GridBagConstraints.BOTH;
        gbc.weightx = 1;
        gbc.weighty = 1;
        gbc.gridwidth = 4;
        add (totalDisplay, gbc);

        //computation display//
        gbc.gridx = 0;
        gbc.gridy = 1;
        gbc.weighty = 0;
        add(computationDisplay, gbc);

        //creating iterator to iterate through all buttons
        Iterator <JButton> buttonRetriever = buttonList.iterator();

        //first row//
        gbc.gridx = 0;
        gbc.gridy = 2;
        gbc.gridwidth=1;
        gbc.fill = GridBagConstraints.BOTH;;
        gbc.weightx = .1;
        gbc.weighty = .1;

        add(buttonRetriever.next(), gbc);

        gbc.gridx = 1;
        add(buttonRetriever.next(), gbc);

        gbc.gridx = 2;
        add(buttonRetriever.next(), gbc);

        gbc.gridx = 3;
        add(buttonRetriever.next(), gbc);

        //second row//

        gbc.gridx = 0;
        gbc.gridy = 3;
        add(buttonRetriever.next(), gbc);

        gbc.gridx = 1;
        add(buttonRetriever.next(), gbc);

        gbc.gridx = 2;
        add(buttonRetriever.next(), gbc);

        gbc.gridx = 3;
        add(buttonRetriever.next(), gbc);

        //third row//

        gbc.gridx = 0;
        gbc.gridy = 4;
        add(buttonRetriever.next(), gbc);

        gbc.gridx = 1;
        add(buttonRetriever.next(), gbc);

        gbc.gridx = 2;
        add(buttonRetriever.next(), gbc);

        gbc.gridx = 3;
        add(buttonRetriever.next(), gbc);

        //fourth row//

        gbc.gridx = 0;
        gbc.gridy = 5;
        add(buttonRetriever.next(), gbc);

        gbc.gridx = 1;
        add(buttonRetriever.next(), gbc);

        gbc.gridx = 2;
        add(buttonRetriever.next(), gbc);

        gbc.gridx = 3;
        add(buttonRetriever.next(), gbc);

        //last row, equals button
        gbc.gridx = 0;
        gbc.gridy = 6;
        gbc.gridwidth=4;
        add(buttonRetriever.next(), gbc);
    }

    private void createButtonList() {
        buttonList = new ArrayList<JButton> ();
        buttonList.add(new JButton("1"));
        buttonList.add(new JButton("2"));
        buttonList.add(new JButton("3"));
        buttonList.add(new JButton("+"));
        buttonList.add(new JButton("4"));
        buttonList.add(new JButton("5"));
        buttonList.add(new JButton("6"));
        buttonList.add(new JButton("-"));
        buttonList.add(new JButton("7"));
        buttonList.add(new JButton("8"));
        buttonList.add(new JButton("9"));
        buttonList.add(new JButton("/"));
        buttonList.add(new JButton("0"));
        buttonList.add(new JButton("."));
        buttonList.add(new JButton("C"));
        buttonList.add(new JButton("*"));
        buttonList.add(new JButton("="));
    }

    private void configureTotalDisplay() {
        totalDisplay.setPreferredSize(new Dimension(600, 40));
        totalDisplay.setColumns(40);
        totalDisplay.setEditable(false);
        totalDisplay.setText("0");
        totalDisplay.setHorizontalAlignment(JTextField.CENTER);

        Font newFont = new Font("SansSerif", Font.PLAIN, 40);
        totalDisplay.setFont(newFont);
    }

    private void setWindowPreferences() {
        setVisible(true);
        setSize(new Dimension(267, 309));
        setDefaultCloseOperation(EXIT_ON_CLOSE);
        setLocation(420, 220);
    }

    public void setCalcButtonListener(ActionListener actionListener) {
        for (JButton button : buttonList){
            button.addActionListener(actionListener);
        }
    }

    public void setComputationDisplayText(String string){
        computationDisplay.setText(string);
    }

    public String getComputationDisplayText(){
        return computationDisplay.getText();
    }

    public void setTotalDisplayText(String string) {
        totalDisplay.setText(string);

    }

    @Override
    public void update(Observable o, Object arg) {
        CalcDisplayData updateObject = (CalcDisplayData) arg;

        if (updateObject.getCurrentInputString()!=null){
            computationDisplay.setText(updateObject.getCurrentInputString());
        }

        if (updateObject.getCurrentTotal()!=null){
            totalDisplay.setText(updateObject.getCurrentTotal());
        }

    }



}

Package Controller

Class Controller

The main controller class which assigns all of the buttons a single listener. The listener uses both ButtonInfo class, which determines the attributes of the button pressed, and a StringInfo class, which determines the attributes of the last character of the computation string, to dictate what happens in every combination of the two.

package Controller;

import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.util.HashMap;

import javax.swing.JButton;

import Model.Model;
import View.View;

public class Controller {

    private Model model;
    private View view;

    public Controller(Model model, View view) {
        this.model = model;
        this.view = view;

        model.addObserver(view);

        view.setCalcButtonListener(new CalcButtonListener());

    }

    class CalcButtonListener implements ActionListener {
        boolean OperationAlreadyHappened = false;

        public void actionPerformed(ActionEvent e) {

            JButton button = (JButton) e.getSource();
            String buttonText = button.getText();
            String computationText = view.getComputationDisplayText();

            ButtonInfo buttonInfo = new ButtonInfo(buttonText);
            StringInfo stringInfo = new StringInfo(computationText);

            if (buttonInfo.isClear) {model.Clear();return;}

            if (stringInfo.isEmpty) {

                if (buttonInfo.isNumber) {setComputationText(computationText + buttonText);
                } else if (buttonInfo.isDot) {setComputationText(computationText + buttonText);
                } else if (buttonInfo.isOperator) {} // do nothing
                else if (buttonInfo.isEquals) {} // do nothing

            } else { // string is NOT empty

                if (stringInfo.isLastCharacterNumber) {

                    if (buttonInfo.isNumber) {
                        if (OperationAlreadyHappened) {setComputationText(buttonText);OperationAlreadyHappened = false;} 
                        else {setComputationText(computationText + buttonText);}
                    }

                    else if (buttonInfo.isOperator) {setComputationText(computationText + buttonText);} 
                    else if (buttonInfo.isDot) {setComputationText(computationText + buttonText);} 
                    else if (buttonInfo.isEquals) {OperationAlreadyHappened = true; performComputation();}
                }

                else if (stringInfo.isLastCharacterOperator) {

                    if (buttonInfo.isNumber) {setComputationText(computationText + buttonText);} 
                    else if (buttonInfo.isOperator) {setComputationText(computationText.substring(0,stringInfo.lastCharIndex) + buttonText);} 
                    else if (buttonInfo.isDot) {setComputationText(computationText + buttonText);} 
                    else if (buttonInfo.isEquals) {} // do nothing

                }

                else if (stringInfo.isLastCharacterDot) {

                    if (buttonInfo.isNumber) {setComputationText(computationText + buttonText);} 
                    else if (buttonInfo.isOperator) {} // do nothing
                    else if (buttonInfo.isDot) {setComputationText(computationText.substring(0,stringInfo.lastCharIndex));} 
                    else if (buttonInfo.isEquals) {} // do nothing

                }

            }

        }

        private void performComputation() {
            model.computeString();
        }

        private void setComputationText(String text) {
            model.setComputationText(text);
        }

    }
}

Class ButtonInfo

Determines whether the button just pressed is an operand, operator, or equals sign.

package Controller;

public class ButtonInfo {

    boolean isOperator;
    boolean isEquals;
    boolean isDot;
    boolean isNumber;
    boolean isClear;

    public ButtonInfo (String buttonText){

        readButtonValue(buttonText);

    }

    private void readButtonValue(String buttonText) {
        switch (buttonText){
            case "C" : isClear = true; break;

            case "1" : isNumber = true; break;
            case "2" : isNumber = true; break;
            case "3" : isNumber = true; break;
            case "4" : isNumber = true; break;
            case "5" : isNumber = true; break;
            case "6" : isNumber = true; break;
            case "7" : isNumber = true; break;
            case "8" : isNumber = true; break;
            case "9" : isNumber = true; break;
            case "0" : isNumber = true; break;

            case "." :  isDot = true; break;

            case "+" : isOperator = true; break;
            case "-" :  isOperator = true; break;
            case "*" : isOperator = true; break;
            case "/" :  isOperator = true; break;

            case "=" : isEquals = true; break;

            default : System.out.println("BUTTON TYPE NOT FOUND");
        }

    }

    public boolean isNumber() {
        return isNumber;
    }

    public boolean isOperator() {
        return isOperator;
    }

    public boolean isEquals() {
        return isEquals;
    }

    public boolean isDot() {
        return isDot;
    }

    public boolean isClear() {
        return isClear;
    }
}

Class StringInfo

Determines the last character of the computation string, and if it is a dot, an operand, an operator, or if the string itself is empty.

package Controller;

public class StringInfo {

boolean isEmpty;
boolean isLastCharacterNumber;
boolean isLastCharacterOperator;    
boolean isLastCharacterDot;
int lastCharIndex;

    public StringInfo(String computationText){

        readStringValue(computationText);

    }

    private void readStringValue(String computationText) {

        if (computationText.equals("")) {
            isEmpty = true;
        } else {
            lastCharIndex = computationText.length()-1;
            String lastChar = computationText.substring(lastCharIndex);

            switch(lastChar){
                case "0" : isLastCharacterNumber = true; break; 
                case "1" : isLastCharacterNumber = true; break; 
                case "2" : isLastCharacterNumber = true; break; 
                case "3" : isLastCharacterNumber = true; break; 
                case "4" : isLastCharacterNumber = true; break; 
                case "5" : isLastCharacterNumber = true; break; 
                case "6" : isLastCharacterNumber = true; break; 
                case "7" : isLastCharacterNumber = true; break; 
                case "8" : isLastCharacterNumber = true; break; 
                case "9" : isLastCharacterNumber = true; break; 

                case "+" : isLastCharacterOperator= true; break; 
                case "-" : isLastCharacterOperator= true; break; 
                case "*" : isLastCharacterOperator= true; break; 
                case "/" : isLastCharacterOperator= true; break; 

                case "." : isLastCharacterDot = true; break; 

                default : System.out.println("Cannot parse last character!");

            }

        }


    }

    public boolean isComputationTextEmpty() {
        return isEmpty;
    }


    public boolean isLastCharacterNumber() {
        return isLastCharacterNumber;
    }

    public boolean isLastCharacterOperator() {
        return isLastCharacterOperator;
    }

    public boolean isLastCharacterDot() {
        return isLastCharacterDot;
    }

    public int getLastCharIndex() {
        return lastCharIndex;
    }

}
\$\endgroup\$
  • \$\begingroup\$ Welcome to CR! Any particular reason why MathOperationsList is not an enum? \$\endgroup\$ – h.j.k. Aug 3 '15 at 9:41
  • 1
    \$\begingroup\$ Welcome to CR! This is such a great first question! Just don't be surprised if it takes a little bit to get an answer, there's a lot of code to sift through. =;)- \$\endgroup\$ – RubberDuck Aug 3 '15 at 9:49
  • \$\begingroup\$ @h.j.k. Can an enum be used to iterate through a list of values? I cycle through all of the operation kinds, one by one in MDAS ordering, when the = button is clicked.. I'm not sure if I can do that with an enum?.... \$\endgroup\$ – steelmonkey Aug 3 '15 at 9:53
  • \$\begingroup\$ Enum.values() returns an Array of enum elements in declaration order, see Enum#valueOf \$\endgroup\$ – Vogel612 Aug 3 '15 at 9:55
  • \$\begingroup\$ @Vogel612 I see. I did not know this. Happy to learn this! :D \$\endgroup\$ – steelmonkey Aug 3 '15 at 10:09
3
\$\begingroup\$

You model should be throwing exceptions instead of printing to the (possibly invisible) console.

if (hasOnlyOneToken) {
    setCurrentTotal(operationTokens.get(0));
} else {
    throw new RuntimeException("uhh.. something went wrong? LOL!");
    // or create a better exception class that fits.
}

Catch it in controller and make it show a JOptionPane.show*Dialog as appropriate.

MathOperationList can be a enum MathOperation. You can get all operations by getting the MathOperation.values() array.

In StringParser there is a regex pattern you split with. It's more efficient to store a static Pattern and reuse that as needed:

private static final Pattern splitPattern = Pattern.compile(String.format(WITH_DELIMITERS, "[*/+-]"));

...
splitPattern.split(string);

The code in ButtonInfo and StringInfo is nearly the exact same. This is a good hint that you can extract it into a single function.

The switch can be simplified by letting the duplicate cases fall through:

switch(lastChar){
    case "0" : case "1" : case "2" : 
    case "3" : case "4" : case "5" : 
    case "6" : case "7" : case "8" : 
    case "9" : isLastCharacterNumber = true; break; 

    case "+" : case "-" : case "*" : 
    case "/" : isLastCharacterOperator= true; break; 

    case "." : isLastCharacterDot = true; break; 

    default : isInvalid = true; break;
    //again don't just print to console but let calling code know there was an error

}

However instead of a series of flags you can instead use an enum to specify what type it is. This lets you use a switch instead of the nested if-else you currently have. This is also a solution for the duplicate code; create a single static public function that returns said enum.

Your view creation requires that the button list is in a particular order. As is you can update it by using a for loop:

for(int i = 0; i< buttonList.size;i++){
    gbc.gridx = i % 4;
    gbc.gridy = 2 + i / 4;
    add(buttonList.get(i), gbc);
}

Extending this you can split the list up in numbers and operators and use 1 loop for the numbers 1-9 like above (with 3 replacing 4): and place the operators and 0 separately.

\$\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.