1

I'm new to Java and I'm trying to make a calculator but I have a problem with the result. It is always "0". I used eclipse with Swing. My codes are like this:

package calculator1;

import java.awt.BorderLayout;

public class MainForm extends JFrame {

private JPanel contentPane;
private JTextField textResult;

/**
 * Launch the application.
 */
public static void main(String[] args) {
    EventQueue.invokeLater(new Runnable() {
        public void run() {
            try {
                MainForm frame = new MainForm();
                frame.setVisible(true);
            } catch (Exception e) {
                e.printStackTrace();
            }
        }
    });
}

double number1=0;
double number2=0;
String islem="";



/**
 * Create the frame.
 */
public MainForm() {
    setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    setBounds(100, 100, 509, 534);
    contentPane = new JPanel();
    contentPane.setBackground(Color.BLACK);
    contentPane.setBorder(new EmptyBorder(5, 5, 5, 5));
    setContentPane(contentPane);

    textResult = new JTextField();
    textResult.setEditable(false);

    textResult.setBounds(5, 5, 486, 42);
    textResult.setColumns(10);

    JButton btn1 = new JButton("1");
    btn1.setBounds(45, 105, 80, 80);
    btn1.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            if(islem.equals(""));
            textResult.setText("");

            textResult.setText(textResult.getText()+"1");
        }
    });

    JButton btn4 = new JButton("4");
    btn4.setBounds(45, 185, 80, 80);
    btn4.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            if(islem.equals(""));
            textResult.setText("");

            textResult.setText(textResult.getText()+"4");
        }
    });

    JButton btn7 = new JButton("7");
    btn7.setBounds(45, 265, 80, 80);
    btn7.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            if(islem.equals(""));
            textResult.setText("");

            textResult.setText(textResult.getText()+"7");
        }
    });


    JButton btn5 = new JButton("5");
    btn5.setBounds(125, 185, 80, 80);
    btn5.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            if(islem.equals(""));
            textResult.setText("");
            textResult.setText(textResult.getText()+"5");
        }
    });

    JButton btn8 = new JButton("8");
    btn8.setBounds(125, 265, 80, 80);
    btn8.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            if(islem.equals(""));
            textResult.setText("");
            textResult.setText(textResult.getText()+"8");
        }
    });

    JButton btn2 = new JButton("2");
    btn2.setBounds(125, 105, 80, 80);
    btn2.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            if(islem.equals(""));
            textResult.setText("");
            textResult.setText(textResult.getText()+"2");
        }
    });

    JButton btn3 = new JButton("3");
    btn3.setBounds(205, 105, 80, 80);
    btn3.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            if(islem.equals(""));
            textResult.setText("");
            textResult.setText(textResult.getText()+"3");
        }
    });

    JButton btn6 = new JButton("6");
    btn6.setBounds(205, 185, 80, 80);
    btn6.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            if(islem.equals(""));
            textResult.setText("");
            textResult.setText(textResult.getText()+"6");
        }
    });

    JButton btn9 = new JButton("9");
    btn9.setForeground(Color.BLACK);
    btn9.setBounds(205, 265, 80, 80);
    btn9.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            if(islem.equals(""));
            textResult.setText("");
            textResult.setText(textResult.getText()+"9");
        }
    });

    JButton btn0 = new JButton("0");
    btn0.setBounds(125, 345, 80, 80);
    btn0.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            if(islem.equals(""));
            textResult.setText("");
            textResult.setText(textResult.getText()+"0");
        }
    });

    JButton btncarp = new JButton("*");
    btncarp.setFont(new Font("Tahoma", Font.PLAIN, 35));
    btncarp.setBounds(382, 60, 80, 80);
    btn0.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            islem="*";
            number1=Double.parseDouble(textResult.getText());

        }
    });

    JButton btnarti = new JButton("+");
    btnarti.setFont(new Font("Tahoma", Font.PLAIN, 35));
    btnarti.setBounds(285, 265, 80, 80);
    btn0.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            islem="+";
            number1=Double.parseDouble(textResult.getText());

        }
    });

    JButton btneksi = new JButton("-");
    btneksi.setFont(new Font("Tahoma", Font.PLAIN, 35));
    btneksi.setBounds(285, 105, 80, 80);
    btn0.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            islem="-";
            number1=Double.parseDouble(textResult.getText());

        }
    });

    JButton btnbolu = new JButton("/");
    btnbolu.setFont(new Font("Tahoma", Font.PLAIN, 35));
    btnbolu.setBounds(285, 185, 80, 80);
    btn0.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            islem="/";
            number1=Double.parseDouble(textResult.getText());

        }
    });

    JButton btnesittir = new JButton("=");
    btnesittir.setForeground(new Color(255, 153, 255));
    btnesittir.setFont(new Font("Tahoma", Font.PLAIN, 35));
    btnesittir.setBounds(285, 345, 80, 80);
    btnesittir.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {


            number1=Double.parseDouble(textResult.getText());
            number2=Double.parseDouble(textResult.getText());




            **double result=0;**

            switch (islem) {
            case "*":
             result=number1*number2;

                break;

            case "/":
                 result=number1/number2;

                break;

            case "+":
                 result=number1+number2;

                break;

            case "-":
                 result=number1-number2;

                break;

            default:


                break;




            } //end switch 


            **textResult.setText(String.valueOf(result));**
            islem="";

        }
    });
    contentPane.setLayout(null);
    contentPane.add(textResult);
    contentPane.add(btn1);
    contentPane.add(btn4);
    contentPane.add(btn7);
    contentPane.add(btn5);
    contentPane.add(btn8);
    contentPane.add(btn2);
    contentPane.add(btn3);
    contentPane.add(btn6);
    contentPane.add(btn9);
    contentPane.add(btn0);
    contentPane.add(btncarp);
    contentPane.add(btnesittir);


    contentPane.add(btnarti);


    contentPane.add(btneksi);


    contentPane.add(btnbolu);




}
}

What can I write instead of "double result=0;" ?

Or maybe I need to change something else?

Andrew Thompson
  • 168,117
  • 40
  • 217
  • 433
  • 1
    What is you want to achieve? Why is `double result = 0;` a problem? – MadProgrammer Sep 10 '14 at 07:52
  • I don't see why you want to change `double result=0;` by something else. The result variable is changed after, so it will not stay at `0`. If it doesn't change, you have a problem somewhere else in your code. – Florent Bayle Sep 10 '14 at 07:52
  • 2
    Avoid using `null` layouts, pixel perfect layouts are an illusion within modern ui design. There are too many factors which affect the individual size of components, none of which you can control. Swing was designed to work with layout managers at the core, discarding these will lead to no end of issues and problems that you will spend more and more time trying to rectify – MadProgrammer Sep 10 '14 at 07:52
  • Thank you for your answers. I don't know where else the problem is if **double result=0;** is OK. – İlayda Unal Sep 10 '14 at 08:04
  • I depends, does it do what you want it to... – MadProgrammer Sep 10 '14 at 08:04
  • That's a strange layout the calculator buttons currently have. Why not go with a layout more like seen in [this answer](http://stackoverflow.com/a/7441804/418556)? – Andrew Thompson Sep 10 '14 at 08:20
  • I haven't finished it yet that's beacuse it is like that. I was going to manage the buttons but i came up with this problem. – İlayda Unal Sep 10 '14 at 08:22
  • @İlaydaUnal I posted up my answer lemme know what it went – Kick Buttowski Sep 10 '14 at 08:23

2 Answers2

2

You seem to have some serious logic issues, take a look at...

number1 = Double.parseDouble(textResult.getText());
number2 = Double.parseDouble(textResult.getText());

How are these two numbers ever going to be different, you've taken the value from same source (textResult)

You are setting islem to "" every time the user presses a numeric key, so if the user presses 1+2, islem is now ""...

The question now is, how to fix it...

You need to change your logic, rather than waiting till the user presses the = button, you could perform each calculation as the enter the values...

This would mean that you would need some kind of method which could maintain a running result and carry out the required calculations

For example...

protected void performOperation(double value) {
    switch (islem) {
        case "*":
            result *= value;
            break;
        case "/":
            result /= value;
            break;
        case "+":
            result += value;
            break;
        case "-":
            result -= value;
            break;
        default:
            result = value;
    } //end switch 
    islem = "";
    textResult.setText(String.valueOf(result));
}

What this does, is takes a value to be calculated, determines what calculation needs to be performed (if any), updates the textResult field and rests the calculate modifier...

You could need to change all your numeric buttons to call this method with the appropriate value, for example...

JButton btn1 = new JButton("1");
btn1.addActionListener(new ActionListener() {
    public void actionPerformed(ActionEvent e) {
        performOperation(1);
    }
});

Now, in your original code, all of your modifier buttons did nothing...this is because you kept registering there actions to btn0 instead, so you need to fix that...

JButton btneksi = new JButton("-");
//...
btn0.addActionListener(new ActionListener() {
    public void actionPerformed(ActionEvent e) {
        islem="-";
        number1=Double.parseDouble(textResult.getText());

    }
});

They should now set the calculation modifier that they need to use...

JButton btncarp = new JButton("*");
//...
btncarp.addActionListener(new ActionListener() {
    public void actionPerformed(ActionEvent e) {
        islem = "*";
    }
});

Which should fix the core issues you seem to be having...

You may also want to take a look at How to Use Actions for more details

MadProgrammer
  • 343,457
  • 22
  • 230
  • 366
  • I believe you make the whole thing little bit confusing for the OP. The op is so novice that cannot choose right name of its variables. :) I do not changing the logic of the op code does not help me the op either :) – Kick Buttowski Sep 10 '14 at 08:33
  • @KickButtowski Well, the OP's core calculation methodology is broken, I could have added a `List` or two, but thought better of it... – MadProgrammer Sep 10 '14 at 08:34
  • I do not think so the major issue that the op had is read the number1 and number2 variables in wrong places. you change the whole logic of the op code, and I am sure the op will end up with huge amount of confusion. please see my answer. :) – Kick Buttowski Sep 10 '14 at 08:36
  • you I always admire you but sometimes is good to bring ourselves level to novice people level to be more helpful :) – Kick Buttowski Sep 10 '14 at 08:38
  • @KickButtowski So move the core functionality to single method is more confusing than having to repeat the same set of operations 10 times, interesting way of thinking... – MadProgrammer Sep 10 '14 at 08:39
  • @KickButtowski We should be lifting people, helping the grow, finding new and better ways to confuse themselves. Besides, centralising the logic and reducing duplication is a core OO principle, what a better time to introduce it ;) – MadProgrammer Sep 10 '14 at 08:40
  • I totally agree with you but the op is confused already and I believe there is no need to make the op more confuse with new material – Kick Buttowski Sep 10 '14 at 08:40
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/60949/discussion-between-kick-buttowski-and-madprogrammer). – Kick Buttowski Sep 10 '14 at 08:41
  • If the OP is already confused, then introducing some basic concepts which will reduce the amount of code and risk of error in duplication (which they did with registering there `ActionListener`s) can't be a bad thing ;) – MadProgrammer Sep 10 '14 at 08:41
  • Thanks a lot. It's my second day on Java and i started with no knowledge. I know it's wrong to try doing something like this before understanding the basics of Java. I tried this codes and i got a syntax error and i couldn't find why. Also because you changed the codes a little, i got a bit confused. For now i want to do it with the same codes i wrote but i will change it later to yours. Thank you again for your help :) – İlayda Unal Sep 10 '14 at 10:02
  • @İlaydaUnal Make sure you keep [Creating a GUI With JFC/Swing](http://docs.oracle.com/javase/tutorial/uiswing/) on hand – MadProgrammer Sep 10 '14 at 10:19
1

I just tried to point out your issues that your code has, and I am sure there are other better ways to solve calculator problem with less code and more concise

First Major issue, your variable naming is Terrible

For example you named * button , btncarp which does not make sense for whoever will read your code.

Second issue, you add your action listener to button 0 in * , / , and - operation

like

    JButton btncarp = new JButton("*");  <---- your button name is btncarp
    ....
    ....
    btn0.addActionListener(new ActionListener() { <----you add action listener to btn0?!!!
        public void actionPerformed(ActionEvent e) {
            ......
            ......
        }
    });

Third issue you never assigned your number1 value , so it turns zero all them time

try this

 btnarti.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            islem = "+";
            number1 = Double.parseDouble(textResult.getText());
            textResult.setText(" ");
        }
    });

Explanation: you will read number1 value and clear the result box and make it ready for second value that you will read inside your operation method

Fourth: you can read your second number which is number2 in = opration ActionListener section and no need to read number1 at that place

like

btnesittir.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e) {
                number2=Double.parseDouble(textResult.getText());
                ....
                  ....
                  }
       }

Please choose some sensible name for your variables

Note: your code is almost ok except the issue that I pointed out and you read your number1 and number2 variables in wrong places

Kick Buttowski
  • 6,709
  • 13
  • 37
  • 58
  • *"you never initialized your number1"* - Actually the OP does, they just do it a crap spot, so it's always equal to `number2`, it's in the `btnesittir` `ActionListener`...great spot for it ;) – MadProgrammer Sep 10 '14 at 08:38
  • 1
    change initialized to assigned. hope it is better – Kick Buttowski Sep 10 '14 at 08:39
  • Thank you very much. My Calculator now works fine and now i will try to make it work with two or more digit numbers. And for the variable naming; i named some of them with my own language that's beacuse it may seem terrible to you:) and i'm sorry for posting it like that – İlayda Unal Sep 10 '14 at 09:54