-1
\$\begingroup\$

I've been working for few days to develop a basic Java Swing Calculator that could Add, Subtract, Multiply or Divide two numbers and also has other options like inverse(1/x) and square root

I would like you to analyze my code that it follows basic OOPS concepts like Abstraction, Encapsulation, Polymorphism etc.

import java.awt.Color;
import java.awt.Font;
import java.awt.GridLayout;
import java.awt.Image;
import java.awt.Toolkit;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

import javax.swing.JButton;
import javax.swing.JPanel;
import javax.swing.JTextField;
import javax.swing.event.ChangeEvent;
import javax.swing.event.ChangeListener;

class Cal extends javax.swing.JFrame implements ActionListener {

    private DigitButton plusminus, b0, b1, b2, b3, b4, b5, b6, b7, b8, b9, bpoint;
    private OpButton add, sub, mul, div, del, clear, inv, sqrt, equal;
    JTextField tf;
    static double a = 0, b = 0, result = 0;
    static int operator = 0;
    private int point;
    private Image icon;

    Cal() {

        super("Calculator");
        point = 0;
        icon = Toolkit.getDefaultToolkit().getImage("icon.png"); // Loading icon image into memory

        /************* Initialising buttons ************/
        b1 = new DigitButton("1");
        b2 = new DigitButton("2");
        b3 = new DigitButton("3");
        b4 = new DigitButton("4");
        b5 = new DigitButton("5");
        b6 = new DigitButton("6");
        b7 = new DigitButton("7");
        b8 = new DigitButton("8");
        b9 = new DigitButton("9");
        b0 = new DigitButton("0");
        bpoint = new DigitButton(".");
        inv = new OpButton("1/x");
        sqrt = new OpButton("√");
        plusminus = new DigitButton("±");
        del = new OpButton("DEL");

        add = new OpButton("+");
        sub = new OpButton("-");
        mul = new OpButton("×");
        div = new OpButton("÷");
        clear = new OpButton("C");
        equal = new OpButton("=");

        // TextField in which result is to be displayed
        tf = new JTextField();
        tf.setEditable(false);
        tf.setFont(new Font("Segoi UI", Font.BOLD, 25)); // Setting font

//      tf.setBackground(Color.DARK_GRAY.darker().darker());
//      tf.setForeground(Color.WHITE.brighter());
        tf.setBackground(Color.WHITE);
        tf.setForeground(Color.BLACK);
        tf.setText("0");

        tf.setBorder(null);

        /********** Setting the location of the buttons ***********/

        JPanel panel = new JPanel();
        JPanel panel2 = new JPanel();
        JPanel panel3 = new JPanel();
        JPanel panel4 = new JPanel();

        panel.add(b7);
        panel.add(b8);
        panel.add(b9);

        panel.add(b4);
        panel.add(b5);
        panel.add(b6);

        panel.add(b1);
        panel.add(b2);
        panel.add(b3);

        panel2.add(plusminus);
        panel2.add(b0);
        panel2.add(bpoint);

        panel3.add(div);
        panel3.add(mul);
        panel3.add(sub);
        panel3.add(add);
        panel3.add(equal);

        panel4.add(clear);
        panel4.add(sqrt);
        panel4.add(inv);
        panel4.add(del);

        GridLayout grid = new GridLayout(3, 3);
        GridLayout grid2 = new GridLayout(1, 2);
        GridLayout grid3 = new GridLayout(5, 1);
        GridLayout grid4 = new GridLayout(1, 4);

        panel.setLayout(grid);
        panel.setBounds(0, 160, 240, 240);

        panel2.setLayout(grid2);
        panel2.setBounds(0, 400, 240, 80);

        panel3.setLayout(grid3);
        panel3.setBounds(240, 160, 80, 320);

        panel4.setLayout(grid4);
        panel4.setBounds(0, 80, 320, 80);

        tf.setBounds(0, 0, 320, 80);

        tf.setHorizontalAlignment(JTextField.RIGHT); // Setting alignment

        /******** Adding Panels to the Frame ***********/
        add(panel);
        add(panel2);
        add(panel3);
        add(panel4);

        add(tf);

        /* Adding ActionListeners to the buttons */
        b0.addActionListener(this);
        b1.addActionListener(this);
        b2.addActionListener(this);
        b3.addActionListener(this);
        b4.addActionListener(this);
        b5.addActionListener(this);
        b6.addActionListener(this);
        b7.addActionListener(this);
        b8.addActionListener(this);
        b9.addActionListener(this);
        bpoint.addActionListener(this);
        clear.addActionListener(this);
        equal.addActionListener(this);
        add.addActionListener(this);
        sub.addActionListener(this);
        mul.addActionListener(this);
        div.addActionListener(this);
        plusminus.addActionListener(this);
        sqrt.addActionListener(this);
        inv.addActionListener(this);
        del.addActionListener(this);

        ////////////////////////////////////////////

        setDefaultCloseOperation(EXIT_ON_CLOSE);
        setResizable(false);
        setIconImage(icon);
        setLocationRelativeTo(null);
        setSize(335, 515);
        setLayout(null);
        setVisible(true);

    }

    private boolean isZero(String Text) {

        if (Text.equals("0"))
            return true;
        else
            return false;
    }

    private void getInput(ActionEvent e) {

        if (tf.getText().equals("∞")) {

        } else if (tf.getText().equals("Invalid input")) {

        } else if (e.getSource().equals(b0)) {
            if (isZero(tf.getText())) {
                tf.setText("0");
            } else {
                tf.setText(tf.getText().concat("0"));
            }
        } else if (e.getSource().equals(b1)) {
            if (isZero(tf.getText())) {
                tf.setText("1");
            } else {
                tf.setText(tf.getText().concat("1"));
            }
        } else if (e.getSource().equals(b2)) {
            if (isZero(tf.getText())) {
                tf.setText("2");
            } else {
                tf.setText(tf.getText().concat("2"));
            }
        } else if (e.getSource().equals(b3)) {
            if (isZero(tf.getText())) {
                tf.setText("3");
            } else {
                tf.setText(tf.getText().concat("3"));
            }
        } else if (e.getSource().equals(b4)) {
            if (isZero(tf.getText())) {
                tf.setText("4");
            } else {
                tf.setText(tf.getText().concat("4"));
            }
        } else if (e.getSource().equals(b5)) {
            if (isZero(tf.getText())) {
                tf.setText("5");
            } else {
                tf.setText(tf.getText().concat("5"));
            }
        } else if (e.getSource().equals(b6)) {
            if (isZero(tf.getText())) {
                tf.setText("6");
            } else {
                tf.setText(tf.getText().concat("6"));
            }
        } else if (e.getSource().equals(b7)) {
            if (isZero(tf.getText())) {
                tf.setText("7");
            } else {
                tf.setText(tf.getText().concat("7"));
            }
        } else if (e.getSource().equals(b8)) {
            if (isZero(tf.getText())) {
                tf.setText("8");
            } else {
                tf.setText(tf.getText().concat("8"));
            }
        } else if (e.getSource().equals(b9)) {
            if (isZero(tf.getText())) {
                tf.setText("9");
            } else {
                tf.setText(tf.getText().concat("9"));
            }
        } else if (e.getSource().equals(bpoint)) {
            if (point < 1) {
                tf.setText(tf.getText().concat("."));
                point = 1;
            } else {

            }
        } else if (e.getSource().equals(sqrt)) {

            double val = Double.parseDouble(tf.getText());

            if (val < 0) {
                tf.setText("Invalid input");
            } else {
                point = 1;
                val = Math.sqrt(val);
                tf.setText(String.valueOf(val));
            }
        } else if (e.getSource().equals(inv)) {

            double val = Double.valueOf(tf.getText());

            if (val == 0) {
                tf.setText("∞");
            } else {
                point = 1;
                val = 1 / val;
                tf.setText(String.valueOf(val));
            }
        } else if (e.getSource().equals(plusminus)) {
            double val = Double.valueOf(tf.getText());

            if (val == 0) {
                tf.setText("0");
            } else {
                point = 1;
                val = -val;
                tf.setText(String.valueOf(val));
            }
        }

    }

    public void actionPerformed(ActionEvent e) {

        if (tf.getText().equals("+") || tf.getText().equals("-") || tf.getText().equals("×")
                || tf.getText().equals("÷")) {

            tf.setText("");
            getInput(e);

        } else if (e.getSource().equals(clear)) {
            point = 0;
            tf.setText("0");
        } else if (e.getSource().equals(del)) {

            String s = tf.getText();
            tf.setText("");

            if (s.equals("∞")) {
                tf.setText("0");
                point = 0;
            } else if (s.equals("Invalid input")) {
                tf.setText("0");
                point = 0;
            } else if (Double.valueOf(s) < 0) {

                if (s.length() < 3) {

                    if (s.matches("-")) {
                        point = 0;
                        tf.setText("0");
                    } else {
                        point = 0;
                        tf.setText("0");
                    }
                } else {

                    for (int i = 0; i < s.length() - 1; i++) {
                        tf.setText(tf.getText() + s.charAt(i));
                    }

                    if (tf.getText().contains(".")) {
                        point = 1;
                    } else {
                        point = 0;
                    }

                }
            } else {

                if (s.length() < 2) {

                    /*
                     * if (s.matches("-")) { Text = "0"; tf.setText(Text); } else {
                     */
                    point = 0;
                    tf.setText("0");
//                  }
                } else {

                    for (int i = 0; i < s.length() - 1; i++) {
                        tf.setText(tf.getText() + s.charAt(i));
                    }

                    if (tf.getText().contains(".")) {
                        point = 1;
                    } else {
                        point = 0;
                    }

                }
            }

        } else if (tf.getText().equals("∞")) {

        } else if (tf.getText().equals("Invalid input")) {

        } else if (e.getSource().equals(add)) {

            a = Double.valueOf(tf.getText());
            operator = 1;
            tf.setText("+");
        } else if (e.getSource().equals(sub)) {

            a = Double.valueOf(tf.getText());
            operator = 2;
            tf.setText("-");
        } else if (e.getSource().equals(mul)) {

            a = Double.valueOf(tf.getText());
            operator = 3;
            tf.setText("×");

        } else if (e.getSource().equals(div)) {

            a = Double.valueOf(tf.getText());
            operator = 4;
            tf.setText("÷");
        } else if (e.getSource().equals(equal)) {

            point = 1;
            tf.setText(calcInput(operator, a));
        } else {
            getInput(e);
        }

    }

    private String calcInput(int operator, double a) {

        b = Double.valueOf(tf.getText());
        String Result;

        switch (operator) {

        case 1:
            result = a + b;
            break;
        case 2:
            result = a - b;
            break;
        case 3:
            result = a * b;
            break;
        case 4:
            if (b != 0)
                result = a / b;
            else {
                result = 0;
                tf.setText("∞");
            }
        }

        if (b == 0)
            Result = "∞";
        else
            Result = String.valueOf(result);

        operator = 0;

        return Result;
    }
}

public class Calc {

    public static void main(String[] args) {

        new Cal();
    }
}

class OpButton extends JButton {

    private Color bgcolor, frcolor;

    OpButton(String op) {

        super(op);
        setBorderPainted(false);
        setFocusPainted(false);

        setFont(new Font("Segoi UI", Font.PLAIN, 22));
        bgcolor = new Color(0, 153, 255);
        frcolor = Color.WHITE;

        setContentAreaFilled(false);
        setOpaque(true);

        setBackground(bgcolor);
        setForeground(frcolor);
        setText(op);

        addChangeListener(new ChangeListener() {

            @Override
            public void stateChanged(ChangeEvent evt) {
                if (getModel().isPressed()) {
                    setBackground(bgcolor.darker().darker());
                } else if (getModel().isRollover()) {
                    setBackground(bgcolor.darker());
                } else {
                    setBackground(bgcolor);
                }
            }
        });
    }

}

class DigitButton extends JButton {

    private Color bgcolor, color2;
    final static long serialVersionUID = 2;

    DigitButton(String digit) {

        super(digit);
        setBorderPainted(false);
        setFocusPainted(false);

        bgcolor = Color.WHITE;
        color2 = new Color(0, 153, 255);

        setFont(new Font("Segoi UI", Font.BOLD, 22));

        setContentAreaFilled(false);
        setOpaque(true);

        // setForeground(Color.BLACK.darker().darker());

        setText(digit);

        setBackground(bgcolor);
        setForeground(color2);

        addChangeListener(new ChangeListener() {

            @Override
            public void stateChanged(ChangeEvent evt) {
                /*
                 * if (getModel().isPressed()) { setBackground(bgcolor.darker().darker()); //
                 * setForeground(Color.WHITE); } else if (getModel().isRollover()) {
                 * setBackground(bgcolor.darker().darker()); // setForeground(Color.WHITE); }
                 * else { // setForeground(color2); setBackground(bgcolor.darker()); }
                 */
                if (getModel().isPressed()) {
                    setBackground(bgcolor.darker().darker());
                    setForeground(Color.WHITE);
                } else if (getModel().isRollover()) {
                    setBackground(bgcolor.darker());
                    setForeground(Color.WHITE);
                } else {
                    setBackground(bgcolor);
                    setForeground(color2);
                }
            }
        });

    }

}
\$\endgroup\$
4
  • 5
    \$\begingroup\$ "Could you please help me to extend it two more that two numbers.", This is not a codewriting service. We can however give feedback on the code you have written \$\endgroup\$ – Ludisposed Feb 20 '19 at 15:48
  • \$\begingroup\$ Look for the source code of GNU shell utils "expr" command. It has everything you need to support expression evaluation. I've ported it to Java for my own needs and it worked perfectly. \$\endgroup\$ – TorbenPutkonen Feb 21 '19 at 6:32
  • \$\begingroup\$ A note, as this is a nice & useful product, but it does not separate things in Model-View-Controller style. And that is a pity: many fields/variables. GUI and calculating mixed; tf.setTexts. Expect some criticism on a nice application. \$\endgroup\$ – Joop Eggen Feb 21 '19 at 9:38
  • 3
    \$\begingroup\$ John, to emphasize on what Ludisposed said, you should edit your question to remove this request if you want your actual code to be reviewed, otherwise your question will probably get closed \$\endgroup\$ – IEatBagels Feb 21 '19 at 14:53
2
\$\begingroup\$

Object Oriented Programming

I would like you to analyze my code that it follows basic OOPS concepts like Abstraction, Encapsulation, Polymorphism etc

In general it is about sending messages (interact with methods) from one object to an other.

The code uses the keyword class but this makes no program object-oriented. To analyze the code and to make it object-oriented takes time and should be done by you.

Some guide lines based the code you provide us:


Model-View-Controller

Currently the business logic and view logic has a high cohesion. With MVC you can decouple the view from the business logic. So it could be imaginable to switch the a view from the "normal" numerals to Roman numerals without changing the business logic.


Code Smell

Don't Make me Think

What is a Cal? Is it a abbreviation for Calculation? Let me take a look into the class.. Ok.. there is a b0, b1, .. they could represent the buttons of a calculator. So Cul stands for Calculator.

Rename Cul to Calculator

Commented-Out Code

Robert C Martin sad

When you see commented-out code, delete it! Don’t worry; the source code control system still remembers it. If anyone really needs it, he or she can go back and check out a previous version. Don’t suffer commented-out code to survive.

A example from the code you provided

/*
* if (getModel().isPressed()) { setBackground(bgcolor.darker().darker()); //
* setForeground(Color.WHITE); } else if (getModel().isRollover()) {
* setBackground(bgcolor.darker().darker()); // setForeground(Color.WHITE); }
* else { // setForeground(color2); setBackground(bgcolor.darker()); }
*/

What does it mean to the person how read it? Should the person ignore it, like a compiler, or shout it be a hind, that this could work? Now one knows..

Comments

Don’t Use a Comment When You Can Use a Function or a Variable

To improve the readability of he code we could wrap the following in methods

/************* Initialising buttons ************/
b1 = new DigitButton("1");
b2 = new DigitButton("2");
// ...
private void initializeButtons() {
    b1 = new DigitButton("1");
    b2 = new DigitButton("2");
    // ...
}

and

// TextField in which result is to be displayed
tf = new JTextField();
tf.setEditable(false);
// ...
private void createTextfield() {
    tf = new JTextField();
    tf.setEditable(false);
    //..
}

till the constructor looks like

Cal() {
    initializeButtons();
    createTextfield();
    giveButtonLocations();
    giveButtonsActionListeners();
}

Follow the Standard

When I first saw the statement Text.equals("0") I thought Text would be a class with a static method equals.

But Text is just a String

private boolean isZero(String Text)

Text should be renamed to text so it is clear that it is a variable.

Empty Blocks

if (tf.getText().equals("∞")) {

} else if (tf.getText().equals("Invalid input")) {

}

As a reader I do not know if there is code missing and if these blocks should be empty that I have to thing about it why they are empty. But if you remove these two empty blocks the logic will be the same.

Duplicate Code

} else if (e.getSource().equals(b0)) {
    if (isZero(tf.getText())) {
        tf.setText("0");
    } else {
        tf.setText(tf.getText().concat("0"));
    }
} else if (e.getSource().equals(b1)) {
    if (isZero(tf.getText())) {
        tf.setText("1");
    } else {
        tf.setText(tf.getText().concat("1"));
    }
} else if (e.getSource().equals(b2)) {
    // ...
}
// ...

Put Buttons Into a Map

class Calculator {
    private Map<DigitButton> digitButtons;

    Calculator() {
        initializeButtons();
        // ...
    }

    private void initializeButtons() {
        Map<DigitButton> digitButtons = new HashMap();
        digitButtons.put("0", new DigitButton("0"));
        digitButtons.put("1", new DigitButton("1"));
        // ..
        this.digitButtons  = digitButtons;
    }
}

A other way would be to accept a map of buttons through the constructor.

class Calculator {
    private Map<String, DigitButton> digitButtons;
    // ...

    Calculator(Map<String, DigitButton> digitButtons) {
        this.digitButtons = digitButtons;
        // ...
    }
}

Replace If-Statements by get

Since a DigitButton is a JButton it is possible to get the text of the button via getText.

private void getInput(ActionEvent e) {
    DigitButton button = digitButtons.get(e.getSource());
    String digit = button.getText();
    if (isZero(tf.getText())) {
        tf.setText(digit);
    } else {
        tf.setText(tf.getText().concat(digit));
    }
    // ...
}

Wrap the Map into a First-Class-Collection

The First Class Collection [FCC] is an idea of the Object Calisthenics.

Any class that contains a collection should contain no other member variables. Each collection gets wrapped in its own class, so now behaviors related to the collection have a home.

class DigitButtonColletion {
    private Map<String, DigitButton> digitButtons;

    DigitButtonColletion(Map<String, DigitButton> digitButtons) {
        this.digitButtons = digitButtons;
    }

    public DigitButton findBySymbol(String symbol) {
        return digitButtons.get(symbol);
    }

    // example for another method
    public DigitButton findAllLessThan(String symbol) {
        //...
    }
}

When you do same for the OpButtons the Calculator could look like

class Calculator {
    private DigitButtonCollection digitButtons;
    private OperationButtonCollection operationButtons;
    // ...

    Calculator(DigitButtonCollection digitButtons,
               OperationButtonCollection operationButtons) {
        this.digitButtons = digitButtons;
        this.operationButtons = operationButtons;
        // ...
    }
}
\$\endgroup\$
1
  • \$\begingroup\$ Thank You @Roman for you detailed review.... :-) As I am a newbie to programming I made many mistakes in the program, but thank you for your guidance and for taking out time to review my code... \$\endgroup\$ – John Aug 10 '19 at 11:15
0
\$\begingroup\$

I suggest to exctract the ActionListener into separate classes and bind them to the Buttons :

button.addActionListener(new AdditionButtonListener())

public class AdditionActionListener implenents ActionListener{} 

I also suggest to separate your view from your Model. Invent A separate Calculator that consists the actual state of your Calculator and use the ActionListeners registered on your GUI buttons to manipulate your calculator state.

In general I would recommend to read about common design patterns so you get A feeling for seperating things and building some kind of logical flow in your application.

\$\endgroup\$
0
\$\begingroup\$

Just more than two terms? How about many terms and operands? You're doing it the hard way, and as you're seeing, it's not easily extensible. You need a different approach.

If you want to evaluate functions of arbitrary complexity, I would suggest the ANTLR4 framework for your expression evaluator. Java is the "native language" of ANTLR4, and by using the Visitor pattern and a simple expression grammar, you can achieve a lot of capability with far less code than you show here.

As an example, see Bart Kiers's Mu project on github on how to do some very powerful things with little code in ANTLR4.

\$\endgroup\$

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