3
\$\begingroup\$

I'm practicing Swing effects. Everything works but for some reason I get a ton of error messages in the command prompt when the fields are empty, despite my attempt to add some exception handling (this has made me realize I need to practice that a lot more).

    /* Author: Luigi Vincent
Practicing Java Swing button and button events; Window that does simple calculation on two numbers
TO DO: Figure out why command prompt show so many messages when fields are empty & stop it
*/

import javax.swing.*;
import java.awt.*;
import java.awt.event.*;
import java.text.DecimalFormat;


public class SimpleCalc extends JFrame {

    SimpleCalc (String name) {
        super(name);

        setLayout(new FlowLayout());

        // Fields for input and output
        JTextField result = new JTextField(10);
        JTextField num1 = new JTextField(5);
        JTextField num2 = new JTextField(5);
        result.setEditable(false);

        // Buttons for operations
        JButton b_add = new JButton("Add");
        JButton b_sub = new JButton("Subtract");
        JButton b_mul = new JButton("Multiply");
        JButton b_div = new JButton("Divide");

        // Listener to handle operations
        ActionListener calculator = new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent e) {
                try {
                    if (num1.getText().length() == 0 || num2.getText().length() == 0) {
                        throw new Exception();
                    }
                }
                catch (Exception ex){
                    JOptionPane.showMessageDialog(null, "Field missing", "Warning", JOptionPane.WARNING_MESSAGE);
                }

                Double alpha = Double.parseDouble(num1.getText());
                Double beta = Double.parseDouble(num2.getText());
                Double omega = null;
                DecimalFormat f = new DecimalFormat("##.00");

                if (e.getSource() == b_add) {
                    omega = alpha + beta;
                }
                else if (e.getSource() == b_sub) {
                    omega = alpha - beta;
                }
                else if (e.getSource() == b_mul) {
                    omega = alpha * beta;
                }
                else if (e.getSource() == b_div) {
                    omega = alpha / beta;
                }
                result.setText(f.format(omega).toString());
            }
        };
       // Register buttons to event listener
        b_add.addActionListener(calculator);
        b_sub.addActionListener(calculator);
        b_mul.addActionListener(calculator);
        b_div.addActionListener(calculator);

       // Add everything to frame + labels    
        add(new JLabel("First Number"));
        add(num1);
        add(new JLabel("Second Number"));
        add(num2);
        add(new JLabel("Result"));
        add(result);
        add(b_add);
        add(b_sub);
        add(b_mul);
        add(b_div);
    }
    public static void main(String[] args) {
        SimpleCalc frame = new SimpleCalc("Simple Calculator");
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.setSize(500, 100);
        frame.setLocationRelativeTo(null);
        frame.setVisible(true);
    }
}

I'm open to any and all improvements possible.

\$\endgroup\$
4
\$\begingroup\$

The major problem I can see is this piece of code :

            try {
                if (num1.getText().length() == 0 || num2.getText().length() == 0) {
                    throw new Exception();
                }
            }
            catch (Exception ex){
                JOptionPane.showMessageDialog(null, "Field missing", "Warning", JOptionPane.WARNING_MESSAGE);
            }

If you throw inside a try-catch, you will not exit the function. Since your condition is num1.getText().length() == 0 when you try to do Double.parseDouble(num1.getTest() well your string is empty so this throw an exception.

You should not use throw and catch to manage an error, not in this way. You should first remove the try-catch and work with a simple if :

if (num1.getText().isEmpty() || num2.getText().isEmpty()) {
    JOptionPane.showMessageDialog(null, "Field missing", "Warning", JOptionPane.WARNING_MESSAGE);
    throw new Exception();
}

I would not suggested that since now you add a complexity to your code.

  • The method now need to declare throws Exception (which is not possible for actionPerformed()), now the caller need to manage this possible outcome.

  • Exception would be too generic.

  • I guess that this case would happen a lot,so throwing an Exception could become heavy for the program.

The real solution is to only return from the method. You've already warned the user that he need to do something (you could be more clear) and you don't want to execute the rest of method.


There is some general notion that are lacking in your code, but I won't cover it all, since I mention it in others answer :

  • There is naming convetions in Java, b_add does not follow it, it should be bAdd (but it could simply be add)

  • You should not extend JFrame, but rather just have a private JFrame, you're not adding any functionnality to JFrame you're only using it. (Composition over inheritance) answer that cover a bit that part

\$\endgroup\$
  • \$\begingroup\$ Mind at least tell me which of those 'other answers' I should look at? Thank you for the input, it was very helpful! I think I was forcing myself to use an Exception when it wasn't necessary, and the parse.double completely eluded me. Quick question about composition over inheritance, if I did it the plain way wouldn't all of this have to be in the main method? (I read somewhere around here that that was bad practice...) \$\endgroup\$ – Legato Jul 15 '14 at 2:23
  • 1
    \$\begingroup\$ No, not at all. You will have a field private JFrame mainFrame; and your constructor will still take a String. You will then make a new JFrame(name) and change all your setLayout and add (in fact all the JFrame call) like this mainFrame.setLayout(""). For another answer to look read this one about generic style : codereview.stackexchange.com/a/55904 \$\endgroup\$ – Marc-Andre Jul 15 '14 at 2:31
  • \$\begingroup\$ Did you need more explanation about he double part or you were good ? And I didn't linked specific answers to not "advertise" myself too much! There are plenty of good answers here that will help you about the generic stuff of Jave! I would recommend you to browse the java and swing tag for more information (reading code can help a lot!) \$\endgroup\$ – Marc-Andre Jul 15 '14 at 2:33

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.