Krdytkyu

can you review this code for me? how do i make it better?

//this is the calculator

import java.awt.*;

import java.awt.event.*;

import javax.swing.*;

import java.util.*;



class dog extends JFrame {

    private JButton n1, n2, n3, n4, n5, n6, n7, n8, n9, n0, add, sub,  mul, div, slo, cls;

    private JPanel panel, panel1;

    private double temp;

    private double solTemp;

    private JTextField srn;

    Boolean addb = false ;

    Boolean subb = false ;

    Boolean divb = false ;

    Boolean mulb = false ;

    String display = "";



    public dog(){

        super("Clac"); //The title

        setLayout(new FlowLayout());

        srn = new JTextField(20);

        add(srn);

        panel = new JPanel();  //numbers buttons

        panel1 = new JPanel (); 

        panel.setLayout(new GridLayout(4, 3));

        panel1.setLayout(new GridLayout(3, 2));

        //the buttons

        n1 = new JButton("1");

        n1.addActionListener(

                new ActionListener() {

                    public void actionPerformed(ActionEvent e){

                         display = srn.getText();

                            srn.setText(display + "1");

                    }

                }

        );

        panel.add(n1);

        n2 = new JButton("2");

        n2.addActionListener(

                new ActionListener() {

                    public void actionPerformed(ActionEvent e){

                         display = srn.getText();

                            srn.setText(display + "2");

                    }

                }

        );

        panel.add(n2);

        n3 = new JButton("3");

        n3.addActionListener(

                new ActionListener() {

                    public void actionPerformed(ActionEvent e){

                         display = srn.getText();

                            srn.setText(display + "3");

                    }

                }

        );

        panel.add(n3);

        n4 = new JButton("4");

        n4.addActionListener(

                new ActionListener() {

                    public void actionPerformed(ActionEvent e){

                         display = srn.getText();

                            srn.setText(display + "4");

                    }

                }

        );

        panel.add(n4);

        n5 = new JButton("5");

        n5.addActionListener(

                new ActionListener() {

                    public void actionPerformed(ActionEvent e){

                         display = srn.getText();

                            srn.setText(display + "5");

                    }

                }

        );

        panel.add(n5);

        n6 = new JButton("6");

        n6.addActionListener(

                new ActionListener() {

                    public void actionPerformed(ActionEvent e){

                         display = srn.getText();

                            srn.setText(display + "6");

                    }

                }

        );

        panel.add(n6);

        n7 = new JButton("7");

        n7.addActionListener(

                new ActionListener() {

                    public void actionPerformed(ActionEvent e){

                         display = srn.getText();

                            srn.setText(display + "7");

                    }

                }

        );

        panel.add(n7);

        n8 = new JButton("8");

        n8.addActionListener(

                new ActionListener() {

                    public void actionPerformed(ActionEvent e){

                         display = srn.getText();

                            srn.setText(display + "8");

                    }

                }

        );

        panel.add(n8);

        n9 = new JButton("9");

        n9.addActionListener(

                new ActionListener() {

                    public void actionPerformed(ActionEvent e){

                         display = srn.getText();

                            srn.setText(display + "9");

                    }

                }

        );

        panel.add(n9);

        n0 = new JButton("0");

        n0.addActionListener(

                new ActionListener() {

                    public void actionPerformed(ActionEvent e){

                         display = srn.getText();

                            srn.setText(display + "0");

                    }

                }

        );

        panel.add(n0);

        cls = new JButton("C");

        cls.addActionListener(

                new ActionListener() {

                    public void actionPerformed(ActionEvent e){

                        srn.setText("");

                        addb = false ;

                        subb = false ;

                        mulb = false ;

                        divb = false ;



                        temp = 0;

                        solTemp =0 ;

                    }

                }

        );

        panel1.add(cls);

        add = new JButton("+");

        add.addActionListener(

                new ActionListener(){

                    public void actionPerformed(ActionEvent e){

                         temp = Double.parseDouble(srn.getText());

                            srn.setText("");

                            addb = true ;

                    }

                }

        );

        panel1.add(add);

        sub = new JButton("-");

        sub.addActionListener(

                new ActionListener(){

                    public void actionPerformed(ActionEvent e){

                         temp = Double.parseDouble(srn.getText());

                            srn.setText("");

                            subb = true ;

                    }

                }

        );

        panel1.add(sub);

        mul = new JButton("*");

        mul.addActionListener(

                new ActionListener(){

                    public void actionPerformed(ActionEvent e){

                         temp = Double.parseDouble(srn.getText());

                            srn.setText("");

                            mulb = true ;

                    }

                }

        );

        panel1.add(mul);

        div = new JButton("/");

        div.addActionListener(

                new ActionListener(){

                    public void actionPerformed(ActionEvent e){

                         temp = Double.parseDouble(srn.getText());

                            srn.setText("");

                            divb = true ;

                    }

                }

        );

        panel1.add(div);    

        slo = new JButton("=");

        slo.addActionListener(

                new ActionListener(){

                    public void actionPerformed(ActionEvent e){

                        solTemp = Double.parseDouble(  srn.getText() );

                        if ( addb == true  )

                            solTemp = solTemp + temp;



                        else if ( subb == true  )

                            solTemp = solTemp - temp;

                        else if ( mulb == true  )

                            solTemp = solTemp * temp;

                        else if ( divb == true  )

                            solTemp = temp / solTemp;

                        srn.setText(  Double.toString( solTemp ) );

                        addb = false;

                        subb = false;

                        mulb = false;

                        divb= false;

                    }

                }

        );

        panel1.add(slo);

        add(panel);

        add(panel1);

    }   

}





// ------------------------------------ Main Class

import javax.swing.*;



class apples {

    public static void main(Stringargs){

        dog ob = new dog();

        ob.setVisible(true);

        ob.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        ob.setSize(250,200);

    }

}

First of all, use clear names for your classes and fields, and apply remarks from @martin-spamer.

Aside of those "styles" remarks, there are others about responsibilities and design because your code mix UI and logic in one class, this is bad for many reasons like maintenance and evolutions but also for testing (Search for Single Responsibility Principle). So what can you do ?

Regarding the UI of your calculator there are some remarks to clena it. We can observe in the new (web) frameworks that the trends is for components. You may consider a "componentization" of your app with a panel for the numbers and another for the operations. Two are better than one because their roles are differents.

For the OperationsPanel, you can reduce the duplication with one factory method to create the button. A factory method can also be used in the NumbersPanel where you can use a for loop to add the buttons from 1 to 9.

For Swing, I like the MVP pattern more than MVC because I can test the presenter logic with a mock of the view. With this pattern, you'll have a CalculatorView interface that contains methods to get and set the text and to listen to operations ('C', '+', '-', ..). A CalculatorPresenter receive this view as constructor parameter and bind itself to it in order to react to all events and drive the business.

class CalculatorPresenter {

  private final ClaculatorView view;



  CalculatorPresenter(CalculatorView view) {

    this.view = view;

    this.view.onClear(()->{

      this.view.setText("");

    });

    this.view.onAdd(()->{

      // ..

      this.view.setText(result);

    });

  }

}

For the logic of your app, you may consider another approach with less states and better separation (easier testing again). In fact, your calculator can be seen as a suit of operations until you request the result (press "=").

abstract class Operation {

  Operation(Integer left) {

    this.left = left;

  }

  abstract Integer apply(Integer right)

}



class Addition extends Operation {

  Integer apply(Integer right) {

    return this.left + right;

  }

}

Doing so you can easily test your operations individually. And from the UI, your presenter has to create the corresponding operation when the user press one operation button.

Your Equation is a stack of Operation that are waiting for the rightmost number to resolve. You just have to read it when the user press the "=" button and resolve your equation.

class Equation {

  Stack<Operation> operations = new Stack<>;

  void push(Operation operation) {

    this.operations.push(operation);

  }

  Integer resolve(Integer x) {

    Integer right = x;

    while ( !operations.isEmpty() ) {

      right = operations.pop().apply(right);

    }

    return right;

  }

}

And that's all. To reset you just have to create a new Equation from your presenter. (as bonus it solve your 1 - 2 = 1 bug)

• Learn and follow the Java naming conventions
  


• Capitalise the first letter of Class names, so Dog not dog; an instance of the class Dog would be called dog.


• Name everything for clarity of purpose from the problem/business domain not a programming domain. Emphasis purpose with naming. Why is a dog providing calculator like functions? Unless this is a toy calculator shaped like a dog this fails the principle of least Astonishment(surprise).


• You have a lot of repeated code, create methods containing the reusable code with parameters. Contentiously extract pieces of functionality into functions, do not use copy and paste, ever.


• Use the command pattern for the actionListener

I think the past two answers cover most of what I'd say however I do have a suggestion to condense the amount of code you've written. Just thinking you could implement the numbers 0-9 in a 1D JButton array. Seems a little pointless creating the buttons individually when they're all so similar. Will save all that code in the constructor too. I'm fairly certain this will work as I did something similar when creating a grid for an implementation of Conways Game of Life.