Any tips for a noob's code?

[Cromulen7]

I wrote a little calculator in Java, but I feel like it's a mess and that I've chosen the worst way to do it.

without any further ado:

Spoiler:

Code: Select all

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

@SuppressWarnings("serial")
public class Calc extends JApplet
{
   
   
   double mem1, mem2;
   char action;
   
   public void init()
   {
      try
      {
         SwingUtilities.invokeAndWait(new Runnable()
         {
            public void run()
            {
               makeGUI();
            }
         });
      } catch(Exception exc)
      {
         System.out.println("Can't create because of " + exc);
      }
   }
   

private void makeGUI ()
   {
      
      //Layout and buttons
      setLayout(new FlowLayout());
      
      final JTextField input = new JTextField (15);
   
      JButton plus = new JButton("+");
      JButton minus = new JButton("-");
      JButton mult = new JButton("*");
      JButton div = new JButton("/");
      JButton eq = new JButton("=");
      JButton period = new JButton(".");
      JButton one = new JButton("1");
      JButton two = new JButton("2");
      JButton three = new JButton("3");
      JButton four = new JButton("4");
      JButton five = new JButton("5");
      JButton six = new JButton("6");
      JButton seven = new JButton("7");
      JButton eight = new JButton("8");
      JButton nine = new JButton("9");
      JButton zero = new JButton("0");
      JButton clear = new JButton("Clear");
      JButton clearAll = new JButton("Clear All");
      
      //listener for clear
      clear.addActionListener(new ActionListener()
      {
         public void actionPerformed(ActionEvent ae)
         {
            String msg = input.getText();
            input.setText(msg.substring(0, msg.length()-1));
         }
      });
      
      //listener for clearAll
            clearAll.addActionListener(new ActionListener()
            {
               public void actionPerformed(ActionEvent ae)
               {
                  input.setText("");
               }
            });
      
      //listener for plus
      plus.addActionListener(new ActionListener()
      {
         public void actionPerformed(ActionEvent ae)
         {
            action = '+';
            mem1 = Double.parseDouble(input.getText());
            input.setText("");
            input.requestFocusInWindow();
         }
      });
      //listener for minus
      minus.addActionListener(new ActionListener()
      {
         public void actionPerformed(ActionEvent ae)
         {
            action = '-';
            mem1 = Double.parseDouble(input.getText());
            input.setText("");
            input.requestFocusInWindow();
         }
      });
      //listener for mult
      mult.addActionListener(new ActionListener()
      {
         public void actionPerformed(ActionEvent ae)
         {
            action = '*';
            mem1 = Double.parseDouble(input.getText());
            input.setText("");
            input.requestFocusInWindow();
         }
      });
      //listener for div
      div.addActionListener(new ActionListener()
      {
         public void actionPerformed(ActionEvent ae)
         {
            action = '/';
            mem1 = Double.parseDouble(input.getText());
            input.setText("");
            input.requestFocusInWindow();
         }
      });
      
      //listener for eq
            eq.addActionListener(new ActionListener()
            {
               public void actionPerformed(ActionEvent ae)
               {
                  mem2 = Double.parseDouble(input.getText());
                  
                  switch(action)
                  {
                  case '+':
                     input.setText(Double.toString(mem1+mem2));
                     break;
                  case '-':
                     input.setText(Double.toString(mem1-mem2));
                     break;
                  case '*':
                     input.setText(Double.toString(mem1*mem2));
                  case '/':
                     input.setText(Double.toString(mem1/mem2));
                  }
               }
            });
            
      //listeners for numbers
      one.addActionListener(new ActionListener()
      {
         public void actionPerformed(ActionEvent ae)
         {
            String num = input.getText();
            input.setText(num + "1");
         }
      });
      
      two.addActionListener(new ActionListener()
      {
         public void actionPerformed(ActionEvent ae)
         {
            String num = input.getText();
            input.setText(num + "2");
         }
      });
      
      three.addActionListener(new ActionListener()
      {
         public void actionPerformed(ActionEvent ae)
         {
            String num = input.getText();
            input.setText(num + "3");
         }
      });
      
      four.addActionListener(new ActionListener()
      {
         public void actionPerformed(ActionEvent ae)
         {
            String num = input.getText();
            input.setText(num + "4");
         }
      });
      
      five.addActionListener(new ActionListener()
      {
         public void actionPerformed(ActionEvent ae)
         {
            String num = input.getText();
            input.setText(num + "5");
         }
      });
      
      six.addActionListener(new ActionListener()
      {
         public void actionPerformed(ActionEvent ae)
         {
            String num = input.getText();
            input.setText(num + "6");
         }
      });
      
      seven.addActionListener(new ActionListener()
      {
         public void actionPerformed(ActionEvent ae)
         {
            String num = input.getText();
            input.setText(num + "7");
         }
      });
      
      eight.addActionListener(new ActionListener()
      {
         public void actionPerformed(ActionEvent ae)
         {
            String num = input.getText();
            input.setText(num + "8");
         }
      });
      
      nine.addActionListener(new ActionListener()
      {
         public void actionPerformed(ActionEvent ae)
         {
            String num = input.getText();
            input.setText(num + "9");
         }
      });
      
      zero.addActionListener(new ActionListener()
      {
         public void actionPerformed(ActionEvent ae)
         {
            String num = input.getText();
            input.setText(num + "0");
         }
      });
      
      period.addActionListener(new ActionListener()
      {
         public void actionPerformed(ActionEvent ae)
         {
            String num = input.getText();
            input.setText(num + ".");
         }
      });
      //end of listeners for numbers
      
      
      add(input);
      clearAll.setPreferredSize(new Dimension(100, 26));
      add(clearAll);
      add(clear);
      add(one);
      add(two);
      add(three);
      add(plus);
      add(four);
      add(five);
      add(six);
      add(minus);
      add(seven);
      add(eight);
      add(nine);
      add(mult);
      add(period);
      add(zero);
      add(eq);
      add(div);      
      
   }
}



I've also uploaded it online, so if you just see what it looks...

http://cromulen7.comule.com/Java/calc2/page.html


so, any tips?

[Pixyn25]

So far, I have this advice:

1. You should actually use an array of buttons instead of "one", "two", ...
For example:

Code: Select all

    a=new JButton[10];
    for(int i=0;i<10;i++) a[i]=new JButton(String.valueOf(i));


2. For the actionlisteners of the number buttons, since you are simply doing the same thing again and again, you should factor it out into a class of its own, say NumberButtonListener, with a constructor so let it know which button it will be listening for, so as to take the appropriate action.

[Cromulen7]

So far, I have this advice:

1. You should actually use an array of buttons instead of "one", "two", ...
For example:

Code: Select all

    a=new JButton[10];
    for(int i=0;i<10;i++) a[i]=new JButton(String.valueOf(i));


2. For the actionlisteners of the number buttons, since you are simply doing the same thing again and again, you should factor it out into a class of its own, say NumberButtonListener, with a constructor so let it know which button it will be listening for, so as to take the appropriate action.

Thanks, I've looked over it, and have been trying to implement it, but I don't quite know where to put what...
Whatever I do, I seem to have problems accessing the TextField "input".

I'm probably just being dense right now. Is there a way to access a variable in a method in another class in a way that isn't making it public? I can't make it public because I've made it final, which I need for everything else to work.

[sourmìlk]

There's a common pattern for that:

Code: Select all

class someClass
{
    private int privateVariable;

    public int getPrivateVariable() //used to get the value of the private variable. cannot alter it's value in the object
    {
        return this.privateVariable;
    }

    public void setPrivateVariable(int set) //used to set the value of the private variable. Allows you to set restrictions on how you set it inside the function.
    {
        this.privateVariable = set;
    }
}


Those methods are called "accessors" and "mutators", or "getters" and "setters". I'll let you figure out which is which.

[Pixyn25]

After trying the applet (by stuffing it into a JFrame), I noticed the following:

1. You are missing a break after your '*' case in your switch statement in the "equals" listener, causing the execution of the division step. So 45*5 turns into 9, and 2*0 turns into Infinity. Whoops.
2. It appears to me that if the user types in something that is not a number, you should be trying to alert him/her. Right now it appears to silently crash the EventQueue thread in the background. (of course, the calculator still works)
3. You should not be using FlowLayout for the number/operator buttons on the calculator.

[Cromulen7]

There's a common pattern for that:

Code: Select all

class someClass
{
    private int privateVariable;

    public int getPrivateVariable() //used to get the value of the private variable. cannot alter it's value in the object
    {
        return this.privateVariable;
    }

    public void setPrivateVariable(int set) //used to set the value of the private variable. Allows you to set restrictions on how you set it inside the function.
    {
        this.privateVariable = set;
    }
}


Those methods are called "accessors" and "mutators", or "getters" and "setters". I'll let you figure out which is which.

Wouldn't that require me to declare(proper terminology?) the JTextField outside GUI()? Isn't that kinda unintuitive and bad?

After trying the applet (by stuffing it into a JFrame), I noticed the following:

1. You are missing a break after your '*' case in your switch statement in the "equals" listener, causing the execution of the division step. So 45*5 turns into 9, and 2*0 turns into Infinity. Whoops.
2. It appears to me that if the user types in something that is not a number, you should be trying to alert him/her. Right now it appears to silently crash the EventQueue thread in the background. (of course, the calculator still works)
3. You should not be using FlowLayout for the number/operator buttons on the calculator.

1. haha thanks, I figured out it wasn't actually dividing right eventually. Perhaps I shouldn't have tested with 46541864 / 684

2. Yes, I just hacked input together and told myself I'd fix it once I properly implement keyboard input.

3. After looking into other Layout Managers, It seems that GridBagLayout is the most flexible one. Would it be okay to just entirely switch to that one?



P.S. Big thanks to both of you!

[chridd]

There's a common pattern for that:

Code: Select all

class someClass
{
    private int privateVariable;

    public int getPrivateVariable() //used to get the value of the private variable. cannot alter it's value in the object
    {
        return this.privateVariable;
    }

    public void setPrivateVariable(int set) //used to set the value of the private variable. Allows you to set restrictions on how you set it inside the function.
    {
        this.privateVariable = set;
    }
}


Those methods are called "accessors" and "mutators", or "getters" and "setters". I'll let you figure out which is which.

Wouldn't that require me to declare(proper terminology?) the JTextField outside GUI()? Isn't that kinda unintuitive and bad?

I don't see why it would be bad. If it's something that you need after makeGUI() finishes, it makes sense to have it be an instance variable (i.e., declared (yes, that's the proper terminology) outside the method).

You can't access a local variable (i.e., declared in a method) outside the method it was declared in. Just moving the variable declaration outside the method will solve the problem of being able to access it from another class; you can even make it public (though whether you should is a different question). Making a variable final doesn't keep it from being public, though if the variable is outside the method it doesn't need to be final anymore (the only time a variable needs to be final is when it's a local variable accessed from an inner class, which is what your new ActionListener() {...}'s are).

[Ben-oni]

There's a common pattern for that:

Code: Select all

class someClass
{
    private int privateVariable;

    public int getPrivateVariable() //used to get the value of the private variable. cannot alter it's value in the object
    {
        return this.privateVariable;
    }

    public void setPrivateVariable(int set) //used to set the value of the private variable. Allows you to set restrictions on how you set it inside the function.
    {
        this.privateVariable = set;
    }
}


Those methods are called "accessors" and "mutators", or "getters" and "setters". I'll let you figure out which is which.

Wouldn't that require me to declare(proper terminology?) the JTextField outside GUI()? Isn't that kinda unintuitive and bad?

There is another viable pattern.

First, you'll need this, because it doesn't look like there's a standard interface that does the job (someone correct me if I'm wrong!):

Code: Select all

interface Functional<T, S> {
  public T call(S v);
}

Define the interface as a private inner interface if you don't want to expose more than necessary.

Now, you can do this:

Code: Select all

Functional<ActionListener, String> numericListenerFactory = new Functional<ActionListener, String>() {
   public ActionListener call(final String digit) {
      return new ActionListener() {
         public void actionPerformed(ActionEvent ae) {
            String num = input.getText();
            input.setText(num + digit);
         }
      };
   }
};
Functional<ActionListener, Character> operationListenerFactory = new Functional<ActionListener, Character>() {
   public ActionListener call(final Character op) {
      return new ActionListener() {
         public void actionPerformed(ActionEvent ae) {
            action = op;
            mem1 = Double.parseDouble(input.getText());
            input.setText("");
            input.requestFocusInWindow();
         }
      };
   }
};


It's not the most elegant thing in the world, but then again, you're working with Java, so your standards can't be too high to begin with. Still, it would be nice to abstract the pattern a little bit (again, someone tell me if this exists somewhere):

Code: Select all

<T> Functional<ActionListener, T> makeListenerFactory(final Functional<Void, T> f) {
   return new Functional<ActionListener, T>() {
      public ActionListener call(final T t) {
         return new ActionListener() {
            public void actionPerformed(ActionEvent ae) {
               f.call(t);
            }
         };
      }
   };
}

Now we can say:

Code: Select all

Functional<ActionListener, Character> operationListenerFactory = makeListenerFactory<Character>(new Functional<Void, Character> () {
   public Void call(final Character op) {
      action = op;
      mem1 = Double.parseDouble(input.getText());
      input.setText("");
      input.requestFocusInWindow();
      return null;
   }
});
Functional<ActionListener, String> numericListenerFactory = makeListenerFactory<String>(new Functional<Void, String> () {
   public Void call(final Character digit) {
      input.setText(input.getText() + digit);
      return null;
   }
});

This is (slightly) more concise. Either way, we can proceed immediately to the conclusion:

Code: Select all

   plus.addActionListener(operationListenerFactory.call('+'));
   minus.addActionListener(operationListenerFactory.call('-'));
   mult.addActionListener(operationListenerFactory.call('*'));
   div.addActionListener(operationListenerFactory.call('/'));
   
   one.addActionListener(numericListenerFactory.call("1"));
   two.addActionListener(numericListenerFactory.call("2"));
   three.addActionListener(numericListenerFactory.call("3"));
   four.addActionListener(numericListenerFactory.call("4"));
   five.addActionListener(numericListenerFactory.call("5"));
   six.addActionListener(numericListenerFactory.call("6"));
   seven.addActionListener(numericListenerFactory.call("7"));
   eight.addActionListener(numericListenerFactory.call("8"));
   nine.addActionListener(numericListenerFactory.call("9"));
   zero.addActionListener(numericListenerFactory.call("0"));
   period.addActionListener(numericListenerFactory.call("."));

Of course, there's a ton more that can be abstracted, but those should be rather easy to spot.

[Cromulen7]

Allright. after some work, here's the state of the project.

I've split everything up into 3 classes

Calc

Spoiler:

Code: Select all

import javax.swing.*;
import java.awt.*;

@SuppressWarnings("serial")
public class Calc extends JApplet
{
   
   
   static double mem1;
   static double mem2;
   static char action;
   static JTextField input = new JTextField (15);
   
   public void init()
   {
//      setInputText("0");
      try
      {
         SwingUtilities.invokeAndWait(new Runnable()
         {
            public void run()
            {
               GUI();
            }
         });
      } catch(Exception exc)
      {
         System.out.println("Can't create because of " + exc);
      }
   }
   

   private void GUI ()
   {
      
      //Layout and buttons
      setLayout(new GridBagLayout());
      GridBagConstraints c = new GridBagConstraints();
   
      JButton plus = new JButton("+");
      JButton minus = new JButton("-");
      JButton mult = new JButton("*");
      JButton div = new JButton("/");
      JButton eq = new JButton("=");
      JButton period = new JButton(".");
      JButton clear = new JButton("Clear");
      JButton clearAll = new JButton("Clear All");
      
      JButton number[] = new JButton[10];   
      for(int i=0;i<10;i++)
      {
         number[i]=new JButton(String.valueOf(i));
         number[i].addActionListener(new NumberButtonListener(i));
      }
      
      c.fill = GridBagConstraints.HORIZONTAL;
      c.gridx = 0;
      c.gridy = 0;
      c.gridwidth = 4;
      c.insets = new Insets(1, 1, 5, 1);
      add(input, c);
      c.fill = GridBagConstraints.BOTH;
      c.gridx = 0;
      c.gridy = 1;
      c.gridwidth = 2;
      c.insets = new Insets(1, 1, 5, 1);
      add(clearAll, c);
      c.gridx = 2;
      c.gridy = 1;
      c.gridwidth = 2;
      c.insets = new Insets(1, 1, 5, 1);
      add(clear, c);
      c.gridx = 0;
      c.gridy = 2;
      c.gridwidth = 1;
      c.insets = new Insets(1, 1, 1, 1);
      add(number[1],c);
      c.gridx = 1;
      c.gridy = 2;
      add(number[2], c);
      c.gridx = 2;
      c.gridy = 2;
      add(number[3], c);
      c.gridx = 3;
      c.gridy = 2;
      add(plus, c);
      c.gridx = 0;
      c.gridy = 3;
      add(number[4], c);
      c.gridx = 1;
      c.gridy = 3;
      add(number[5], c);
      c.gridx = 2;
      c.gridy = 3;
      add(number[6], c);
      c.gridx = 3;
      c.gridy = 3;
      add(minus, c);
      c.gridx = 0;
      c.gridy = 4;
      add(number[7], c);
      c.gridx = 1;
      c.gridy = 4;
      add(number[8], c);
      c.gridx = 2;
      c.gridy = 4;
      add(number[9], c);
      c.gridx = 3;
      c.gridy = 4;
      add(mult, c);
      c.gridx = 0;
      c.gridy = 5;
      add(period, c);
      c.gridx = 1;
      c.gridy = 5;
      add(number[0], c);
      c.gridx = 2;
      c.gridy = 5;
      add(eq, c);
      c.gridx = 3;
      c.gridy = 5;
      add(div, c);   
      
      //listener for clear
      clear.addActionListener(new OperationListener());
      
      //listener for clearAll
      clearAll.addActionListener(new OperationListener());
      
      //listener for plus
      plus.addActionListener(new OperationListener());
      
      //listener for minus
      minus.addActionListener(new OperationListener());
      
      //listener for mult
      mult.addActionListener(new OperationListener());
      
      //listener for div
      div.addActionListener(new OperationListener());
      
      //listener for eq
            eq.addActionListener(new OperationListener());
               
   }

   //Setter and getter for TExtField input
   public static void setInputText(String a)
   {
      input.setText(a);
   }
   
   public static String getInputText()
   {
      return input.getText();
   }
   
   //Setter and getter for action
   public static void setAction(char a)
   {
      action = a;
   }
   
   public static char getAction()
   {
      return action;
   }
   
   //Setter and getter for mem1
   public static void setMem1(double a)
   {
      mem1 = a;
   }
   
   public static double getMem1()
   {
      return mem1;
   }
   
   //Setter and getter for mem2
   public static void setMem2(double a)
   {
      mem2 = a;
   }
   
   public static double getMem2()
   {
      return mem2;
   }
   
   
   
   
}



OperationListener

Spoiler:

Code: Select all

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

//Listener for +,-,etc... (Every button that isn't a number)
public class OperationListener implements ActionListener{
   
   public void actionPerformed(ActionEvent ae)
   {
      String command = ae.getActionCommand();
      switch(command)
      {
      case "Clear All":
         Calc.setInputText("");
         break;
      case "Clear":
         Calc.setInputText(Calc.getInputText().substring(0, Calc.getInputText().length() - 1));
         break;
      case "+":
         Calc.setAction('+');
         Calc.setMem1(Double.parseDouble(Calc.getInputText()));
         Calc.setInputText("");
         break;
      case "-":
         Calc.setAction('-');
         Calc.setMem1(Double.parseDouble(Calc.getInputText()));
         Calc.setInputText("");
         break;
      case "*":
         Calc.setAction('*');
         Calc.setMem1(Double.parseDouble(Calc.getInputText()));
         Calc.setInputText("");
         break;
      case "/":
         Calc.setAction('/');
         Calc.setMem1(Double.parseDouble(Calc.getInputText()));
         Calc.setInputText("");
         break;
      case "=":
         try
         {
            Calc.setMem2(Double.parseDouble(Calc.getInputText()));
         } catch (NumberFormatException exc)
         {
//            Calc.setInputText("Invalid number");
         }
         
         switch(Calc.getAction())
         {
         case '+':
            Calc.setInputText(Double.toString(Calc.getMem1() + Calc.getMem2()));
            break;
         case '-':
            Calc.setInputText(Double.toString(Calc.getMem1() - Calc.getMem2()));
            break;
         case '*':
            Calc.setInputText(Double.toString(Calc.getMem1() * Calc.getMem2()));
            break;
         case '/':
            Calc.setInputText(Double.toString(Calc.getMem1() / Calc.getMem2()));
            break;
         case ' ':
            break;
         }
      }
   }
   
}


NumberButtonListener

Spoiler:

Code: Select all

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


public class NumberButtonListener implements ActionListener
{
   int number;
   NumberButtonListener(int a)
   {
      number = a;
   }

   public void actionPerformed(ActionEvent ae)
   {
      Calc.setInputText(Calc.getInputText() + String.valueOf(number));
   }
}


Right now I feel like project is in a much better state. Stuff's all neat and organized.

@Ben-oni
Thanks for all the code, but I think it's unnecessarily complicated for this project.


If any of you just want to see the thing:
http://cromulen7.comule.com/Java/calc3/page.html


just one more question... how do should I go about implementing keyboard input?
I've been searching and there's like half a dozen different ways to listen to the keyboard.

[Ben-oni]

Right now I feel like project is in a much better state. Stuff's all neat and organized.

@Ben-oni
Thanks for all the code, but I think it's unnecessarily complicated for this project.

You've actually taken a step backward in design. Before, you had the meaning of a button pleasantly close to the definition of the button. Now, not only is the meaning in a different method, it's in a whole different source file. It's desirable to be able to say makeNumberButton('1') and have the meaning of that definition easily accessible, especially if it will be different from, say, makeZeroButton(). Using switches in listeners is worse than having anonymous methods for them. Switches, are, for one, less extensible. If you need to change something, it's easier if everything related to a particular feature is closer together in code, and disentangled from unrelated features.

Also, the getters and setters are just ugly. Toss 'em. Note that mem1, mem2, and action are now declared in Calc, a class that doesn't use them at all. This is highly undesirable. But wait, it gets worse. They're static members of the class, so you can't possibly create a second Calc in the same runtime!

[Cromulen7]

Right now I feel like project is in a much better state. Stuff's all neat and organized.

@Ben-oni
Thanks for all the code, but I think it's unnecessarily complicated for this project.

You've actually taken a step backward in design. Before, you had the meaning of a button pleasantly close to the definition of the button. Now, not only is the meaning in a different method, it's in a whole different source file. It's desirable to be able to say makeNumberButton('1') and have the meaning of that definition easily accessible, especially if it will be different from, say, makeZeroButton(). Using switches in listeners is worse than having anonymous methods for them. Switches, are, for one, less extensible. If you need to change something, it's easier if everything related to a particular feature is closer together in code, and disentangled from unrelated features.

Also, the getters and setters are just ugly. Toss 'em. Note that mem1, mem2, and action are now declared in Calc, a class that doesn't use them at all. This is highly undesirable. But wait, it gets worse. They're static members of the class, so you can't possibly create a second Calc in the same runtime!

Well, when you put it THAT way, I'll look into doing what you said. After all, this is a learning project, and best practices are important.

[Yakk]

A single switch statement like that is an anti-pattern. It ends up being really annoying to maintain.

In C#, you'd use delegates. In C++11, you'd use lambdas.

You have lots of copy-paste code in there. Refactor it.
Change this:

Code: Select all

Calc.setAction('*');
Calc.setMem1(Double.parseDouble(Calc.getInputText()));
Calc.setInputText("");

to:

Code: Select all

Calc.pushOperation('*');

when you do the same thing many times, don't repeat the code.

Ben-oni's template/generic stuff is overkill for such a small project by a beginner. Simplifying it by having only one interface to your function-class makes things nicer.

This then gives us a PlusListener object that looks a bit like this:

Code: Select all

public class PlusListener implements ActionListener { public void actionPerformed(ActionEvent ae) { Calc.pushOperation("+"); }; };

Now, we still need to figure out a better way to get at that Calc than using global state. Sadly, that requires writing a bit of glue code as far as I can tell -- IIRC, Java does not allow implementation inheritance? The point of this is that at the time you create the listener, you know which branch the listener is going to execute. Deferring that to a switch statement is both confusing and inefficient.

Another issue is that your interface to Calc is a bunch of raw accessors to static data.

Instead, I'd have the concept of a stack of input, and you push new input on that stack, then you "rectify" the stack (so 272,9 becomes 2729, and 2,+3,= becomes 5 -- well, in your case, = means that you push everything to the left of it into a string, then evaluate it...)

Improvements, like 3+-2 becoming 3-2 happens during rectification.

[sam_i_am]

Here's my feedback in no particular order

the `*` symbol divides. this is because you don't have a `break;` statement at the end of your multiplication case.

when you hit "clear all" `mem1`, `action` are not reset, so entering in a number and continuing to hit `=` does the operation again. This is not how most people expect their calculator to work.

`mem1` is not very descriptive, and `mem2` is better suited to be a local variable in your `eq` listener

the "clear" button erases the last letter you typed. instead of "clear" you should have "backspace"

Other than that, I really don't have too much of a problem with your code.

It is very simple, which is a good thing.

It's not very object oriented; a project this small doesn't have to be object oriented, but object oriented design would be a good thing to learn, as it really comes in handy for large projects. as a learning exercise, it would be worth your time to make it object oriented.

[Cromulen7]

First of all I would like to thank everyone who replied. You've all been most helpful.

I've made a couple of changes but the code mostly remains as in my previous post.
Now, I'll be gone for a week so It's probably gonna stay that way for a while.
If I get the time I'd like to implement the queue (It's half done now.) and store the numbers and operators buttons in an array, so that button adding part isn't so long and repeating.

Even though I've learned a lot through the last few days, I've also learned how much I don't know.
I'll keep working on my skills, and hopefully one day I'll be the one helping.