8
\$\begingroup\$

I'm a beginner to Java and OOP, and I would really like to learn how to better organize my code! Over the course of a month or two, I made a calculator program with little functions I thought of here and there with a few small jokes built into it. After looking at it a second time, I realized that it is extremely poorly formatted and almost incomprehensible. I would like to ask some more experienced programmers to point me in the right direction on what I should do to fix it (for example, what things can I turn into objects, where can I compartmentalize, etc).

Also, before anyone asks, NO this is NOT homework, it is the product of my own crack at teaching myself Java (probably why it is not working too well).

    // This is the original Calculator code without objects in a single class. not really efficient...

package randomClasses;

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

@SuppressWarnings("serial")
public class CalcClass extends JFrame implements ActionListener {


JPanel[] row = new JPanel[6];
JButton[] button = new JButton[21];
String[] buttonString = {"7", "8", "9", "+",
                         "4", "5", "6", "-",
                         "1", "2", "3", "*",
                         ".", "/", "C", "v",
                         "+/-", "=", "0", 
                         "Parabola", "x^y"};
int[] dimW = {300, 45, 100, 90, 180};
int[] dimH = {35, 40};
Dimension displayDimension = new Dimension(dimW[0], dimH[0]);
Dimension regularDimension = new Dimension(dimW[1], dimH[1]);
Dimension rColumnDimension = new Dimension(dimW[2], dimH[1]);
Dimension zeroButDimension = new Dimension(dimW[3], dimH[1]);
Dimension parabolaDimension = new Dimension(dimW[4], dimH[0]);

//formatting variables
int var = 0;
double x = 0;
String stor = "";
boolean initial = true;

//variables for Parabola function
int countEquals_parab = 0;
double Angle = 0;
double Vi = 0;
double Vx = 0;
double Vy = 0;
double T_max = 0;
double Y_displ = 0;
double X_displ = 0;
double h = 0;
double k = 0;
double a_parab = 0;
boolean parabComplete = true;
boolean parabola = false;
DecimalFormat df = new DecimalFormat("#######.#####");

//variables for addressing illegal typing issues
boolean typeNum = true;
boolean typeDot = true;

JFrame frame; //for parabolaInstructions

//original calculator variables
boolean[] function = new boolean[5];
double[] temporary = {0, 0}; //store on screen values
double result = 0; //store result
public JTextArea display = new JTextArea(1,20);
Font font = new Font("Times new Roman", Font.BOLD, 14);

CalcClass() {
   super("CalcClass");
   setDesign();
   setSize(380, 300);
   setResizable(false);
   setDefaultCloseOperation(EXIT_ON_CLOSE);
   GridLayout grid = new GridLayout(6,5);
   setLayout(grid);

  for(int i = 0; i < 5; i++)
      function[i] = false;

  FlowLayout f1 = new FlowLayout(FlowLayout.CENTER);
  FlowLayout f2 = new FlowLayout(FlowLayout.CENTER,1,1);
  for(int i = 0; i < 6; i++)
      row[i] = new JPanel();
  row[0].setLayout(f1);
  for(int i = 1; i < 6; i++)
      row[i].setLayout(f2);

  for(int i = 0; i < 21; i++) {
      button[i] = new JButton();
      button[i].setText(buttonString[i]);
      button[i].setFont(font);
      button[i].addActionListener(this);
  }

  display.setFont(font);
  display.setEditable(false);
  display.setPreferredSize(displayDimension);

  for(int i = 0; i < 14; i++)
      button[i].setPreferredSize(regularDimension);
  for(int i = 14; i < 18; i++)
      button[i].setPreferredSize(rColumnDimension);
  button[18].setPreferredSize(zeroButDimension);
  button[19].setPreferredSize(parabolaDimension);
  button[20].setPreferredSize(rColumnDimension);

  row[0].add(display);
  add(row[0]);

  for(int i = 0; i < 4; i++)
      row[1].add(button[i]);
  row[1].add(button[14]);
  add(row[1]);

  for(int i = 4; i < 8; i++)
      row[2].add(button[i]);
  row[2].add(button[15]);
  add(row[2]);

  for(int i = 8; i < 12; i++)
      row[3].add(button[i]);
  row[3].add(button[16]);
  add(row[3]);

  row[4].add(button[18]);
  for(int i = 12; i < 14; i++)
      row[4].add(button[i]);
  row[4].add(button[17]);
  add(row[4]);


  row[5].add(button[19]);
  row[5].add(button[20]);
  add(row[5]);

  setVisible(true); 
}

public void getSqrt() {
    stor = "";
    initial = true;
    try {
        double value = Double.parseDouble(display.getText());
        if(value == -100)
            format("John's Girlfriend");
        else {
        value = Math.sqrt(Double.parseDouble(display.getText())); //create a value for variable, and use Maths square root to find the value
        format(Double.toString(value)); //Sets display to new value
        }
    } catch(NumberFormatException e){
    }
    typeDot = false;
    typeNum = false;
}

public void getPosNeg() {
    stor = "";
    initial = true;
    try {
        double value = Double.parseDouble(display.getText()); //again creating a variable for current value
        if(value !=0) { //if value is not equal to zero
            value = (-1) * value; //multiplied by -1 to change the sign
            format(Double.toString(value)); //Sets display to new value
        }
        else{
        }
    } catch(NumberFormatException e) {}

}

public void getResult(){
        temporary[1] = Double.parseDouble(display.getText());
        String temp0 = Double.toString(temporary[0]);
        String temp1 = Double.toString(temporary[1]);

        try {
            if(temp0.contains("-")) {
                String[] temp00 = temp0.split("-", 2);
                temporary[0] = (Double.parseDouble(temp00[1]) * -1);
            }
            if(temp1.contains("-")) {
                String[] temp11 = temp1.split("-", 2);
                temporary[1] = (Double.parseDouble(temp11[1]) * -1);
            }
        } catch(ArrayIndexOutOfBoundsException e) {
        }

    try{
        functions();
        clear();
        format(Double.toString(result));//display has a result
        for(int i = 0; i < 5; i++)
            function[i] = false; //set all functions to false
    }catch(NumberFormatException e) {}
    typeNum = false;
}

public void functions(){
    if(function[2] == true) //multiplication 
        result = temporary[0] * temporary[1];
    else if(function[3] == true) //division
        result = temporary[0] / temporary[1];
    else if(function[0] == true) //addition
        result = temporary[0] + temporary[1];
    else if(function[1] == true) //subtraction;
        result = temporary[0] - temporary[1];
    else if(function[4] == true)
        result = Math.pow(temporary[0], temporary[1]);
    else result = temporary[1];
    }

double a_quadratic = 0;
double b = 0;
double c = 0;

double x1 = 0;
double x2 = 0;
double discr = 0;
int countEquals_quadratic = 0;

public void quadraticFormula(){
    if(countEquals_parab == 0){
        a_quadratic = Double.parseDouble(display.getText());
        clear();
        display.setText("b = ");
    }
    if(countEquals_parab == 1){
        b = Double.parseDouble(display.getText());
        display.setText("c = ");
    }
    if(countEquals_parab == 2){
        c = Double.parseDouble(display.getText());
        discr = (Math.pow(b,2) - 4*a_quadratic*c); //stores the value of the discriminant


        if (discr >= 0){
            x1 = (-b + Math.sqrt(b*b-4*a_quadratic*c))/(2*a_quadratic);
            x2 = (-b - Math.sqrt(b*b-4*a_quadratic*c))/(2*a_quadratic);
        }
    }

}

public void parabolaButton(){
    double G = 9.81;

    if(countEquals_parab == 0){
            Vi = Double.parseDouble(display.getText());
            clear();
            display.setText("Angle of release: ");
        }
    if(countEquals_parab == 1){
        Angle = Double.parseDouble(display.getText());

        if ((Angle > 90.0) || (Angle < 0.0)){
            display.setText("Sorry, not a valid angle");
            countEquals_parab = 3;
        }

    else{

        Angle = (Math.PI / 180.0) * Angle;  //converting degrees into radians

        Vx = Vi * Math.cos(Angle); //Calculating x component
        Vy = Vi * Math.sin(Angle); //Calculating y component

        //Finding time
        T_max = Vy / G; //time to max height

        //Calculating vertex coordinates
        Y_displ = (Vy * Vy / (2 * G));
        X_displ = Vx * T_max;

        //finding a
        a_parab = (-Y_displ) / (X_displ * X_displ);

        display.setText("The equation of the parabola is \ny = " + df.format(a_parab)
            + "(x - " + df.format(h) + ")^2 + " + df.format(k));
    }
    }
        if(countEquals_parab == 2){
            display.setText("Time to get to max height = " + df.format(T_max));
        }
        if(countEquals_parab == 3){
            clearFunction();
            countEquals_parab = -1;
            parabola = false;
            parabComplete = true;
        }
    countEquals_parab++;
}

public void var(){
    var++;
    if(var > 8) var = 1;
    if(var == 1) format("x");
}

public final void setDesign() {
    try {
        UIManager.setLookAndFeel(
        "com.sun.java.swing.plaf.nimbus.NimbusLookAndFeel");
    } catch(Exception e) {
    }
}

public void format(String get){
    //get stores the incoming values temporarily
    //get is transferred to a new value for permanent storage
    //print the permanent storage value
    //new number is added, stored temporarily in get
    //get is added to permanent storage
    //print permanent storage value
    double spaceFix = 0;

    if(initial == true){
     stor = get;
     initial = false;
    }
    else if(initial == false){
        stor = stor + get;
    }
    spaceFix = stor.length()/4;
    int numberOfSpaces = 56 - stor.length() + (int)spaceFix;
    String format = String.format("%" + numberOfSpaces + "s", stor);
    display.setText(format);
}

@Override
public void actionPerformed(ActionEvent ae) {
    if(ae.getSource() == button[0]) numberButtons("7");

    if(ae.getSource() == button[1]) numberButtons("8");

    if(ae.getSource() == button[2]) numberButtons("9");

    if(ae.getSource() == button[3]) operatorButtons(0); //add function[0]

    if(ae.getSource() == button[4]) numberButtons("4");

    if(ae.getSource() == button[5]) numberButtons("5");

    if(ae.getSource() == button[6]) numberButtons("6");

    if(ae.getSource() == button[7]) operatorButtons(1); //subtract function[1]

    if(ae.getSource() == button[8]) numberButtons("1");

    if(ae.getSource() == button[9]) numberButtons("2");

    if(ae.getSource() == button[10]) numberButtons("3");

    if(ae.getSource() == button[11]) operatorButtons(2); //multiplication function[2]

    if(ae.getSource() == button[12]) {
        if(typeDot == false){}
         else{
        numberButtons(".");
        typeDot = false;}
    }

    if(ae.getSource() == button[13]) operatorButtons(3); //divide function[3]

    if(ae.getSource() == button[14]) {
        clearFunction();
        parabola = false;
        parabComplete = true;
    }

    if(ae.getSource() == button[15]) getSqrt();

    if(ae.getSource() == button[16]) getPosNeg();

    if((ae.getSource() == button[17]) && display.getText().equals("")) {}
    else if((ae.getSource() == button[17]) && (parabola == false)) getResult();
    else if ((ae.getSource() == button[17]) && (parabola == true)) parabolaButton();

    if(ae.getSource() == button[18]) numberButtons("0");

    if(ae.getSource() == button[19]) {
        clearFunction();
        parabolaInstructions();
        parabola = true;
        parabComplete = false;
        display.setText("Initial velocity: ");
    }

    if(ae.getSource() == button[20]) operatorButtons(4);//powerFunction();
}

public void parabolaInstructions(){
    //Create the dialog.
                    final JDialog dialog = new JDialog(frame,
                                                       "How to use the Parabola function");

                    //Add contents to it. It must have a close button,
                    //since some L&Fs (notably Java/Metal) don't provide one
                    //in the window decorations for dialogs.
                    JLabel label = new JLabel("<html><p align=center>"
                + "Step 1:  Type in the initial velocity and press the \"=\" button<br>"
                + "Step 2:  Type in the angle of Release (make sure that it is between 0 and 90)<br>"
                + "Step 3:  Press the \"=\" button to scroll through the results<br>"
                + "Step 4:  Profit");
                    label.setHorizontalAlignment(JLabel.CENTER);
                    Font font = label.getFont();
                    label.setFont(label.getFont().deriveFont(font.PLAIN,
                                                             14.0f));

                    JButton closeButton = new JButton("Ok");
                    closeButton.addActionListener(new ActionListener() {
                        public void actionPerformed(ActionEvent e) {
                            dialog.setVisible(false);
                            dialog.dispose();
                        }
                    });
                    JPanel closePanel = new JPanel();
                    closePanel.setLayout(new BoxLayout(closePanel,
                                                       BoxLayout.LINE_AXIS));
                    closePanel.add(Box.createHorizontalGlue());
                    closePanel.add(closeButton);
                    closePanel.setBorder(BorderFactory.
                        createEmptyBorder(0,0,5,5));

                    JPanel contentPane = new JPanel(new BorderLayout());
                    contentPane.add(label, BorderLayout.CENTER);
                    contentPane.add(closePanel, BorderLayout.PAGE_END);
                    contentPane.setOpaque(true);
                    dialog.setContentPane(contentPane);

                    //Show it.
                    dialog.setSize(new Dimension(400, 200));
                    dialog.setLocationRelativeTo(frame);
                    dialog.setVisible(true);
                }

public void numberButtons(String i) {
        if(typeNum == false) {
            display.setText("");
            format(i);
        } else {
            format(i);
        }
        typeNum = true;
}

public void operatorButtons(int funct){
    if (display.getText().equals("")) {}
    else {
    temporary[0] = Double.parseDouble(display.getText());
        function[funct] = true;
        clear();}
}

public void clearFunction() {
    clear();
    try {
        for(int i = 0; i < 5; i++)
            function[i] = false;
        for(int i = 0; i < 2; i++)
            temporary[i] = 0;
    } catch(NullPointerException e){}

    //For parabola()
    Vi = 0;
    Vx = 0;
    Vy = 0;
    T_max = 0;
    Y_displ = 0;
    X_displ = 0;
    h = 0;
    k = 0;
    a_parab = 0;
}

public void clear(){
    display.setText("");
    stor = "";
    typeDot = true;
    initial = true;
}

public static void main(String[] arguments) {
        CalcClass c = new CalcClass();
    }
}

Now you've seen my mess... I sort-of know what I should do, and YES I did some research, but I feel it would be much easier to learn organization through example or a nice push than it would be from reading articles that tell you ultra-hypothetical or loosely-analogous examples of what objects are.

Note: I tried using methods to organize and my class looks much better than what it did (I also made the whole thing an object to be called upon at the bottom which is pretty much useless).

\$\endgroup\$
4
  • \$\begingroup\$ Welcome to Code Review. Your original title and introductory paragraph were redundant — we will automatically address those issues on every Code Review question. \$\endgroup\$ Commented May 31, 2015 at 20:15
  • \$\begingroup\$ What's with the Easter egg? Are you trying to say that John has an imaginary girlfriend? \$\endgroup\$ Commented May 31, 2015 at 20:16
  • \$\begingroup\$ No, it's very well the point. On this site, we don't like titles such as the original one. Otherwise, every title would be about the same, which would look far less interesting. Any aspect of the code can be reviewed, not just your own preferences. \$\endgroup\$
    – Jamal
    Commented May 31, 2015 at 20:16
  • 3
    \$\begingroup\$ @200_success The Easter egg is a reference to a joke I told to a friend (john) a while back... I said "John, your girlfriend is like the square root of -100: shes a 10 but shes imaginary". \$\endgroup\$
    – Hobopowers
    Commented May 31, 2015 at 20:18

4 Answers 4

10
\$\begingroup\$

Just a few tips:

  • Let your IDE format the code, it's zero work and the result is consistent.
  • You should always separate the UI and the computation. Here, it may be a bit pointless as the computation is much simpler than the UI, but still it allows you to separate things. And when you want to make an Android (or console) version, you have a basis. It also allows you to test your logic (testing UI is possible, but hard and rarely done).

 

 import java.awt.*;

Don't use star imports. let you IDE take care of it.

public class CalcClass extends JFrame implements ActionListener {

This is fine, but a separate ActionListener would allow you to structure the code more.

JPanel[] row = new JPanel[6];

Consider using miglayout which can do it without the panels.

String[] buttonString = {"7", "8", "9", "+", ...}

Good, but consider adding more information here. Your buttons have not only a string, but also some action and you may want to use an enum or a class to specify it.

Concerning naming, how long can you remember what rColumnDimension, zeroButDimension, stor, and x mean? Names like countEquals_parab, Angle, Vi, T_max violate Java conventions (starting lowercase, no underscores). Other names are mostly fine.

for(int i = 0; i < 5; i++)
    function[i] = false;

This should look like

for (int i = 0; i < function.length; i++) {
    function[i] = false;
}

Note the spacing, brackets, and the eliminated magic constant.

for(int i = 0; i < 14; i++)
    button[i].setPreferredSize(regularDimension);
for(int i = 14; i < 18; i++)
    button[i].setPreferredSize(rColumnDimension);
button[18].setPreferredSize(zeroButDimension);
button[19].setPreferredSize(parabolaDimension);
button[20].setPreferredSize(rColumnDimension);

I'm throwing a TooManyMagicNumbersException. What about

private Dimension buttonDimension(int index) {
     if (index < REGULAR_BUTTONS_COUNT) {
         return regularDimension;
     } else if (index < WHATEVER) {
         return rColumnDimension;
     }
     switch (index) {
         case ZERO_BUTT_INDEX: return zeroButDimension;
         ...
     }
     throw new IllegalArgumentException("Unknown index: " + index);
}

and

button[i].setPreferredSize(buttonDimension(i));

added to the other loop?

Then even more magic numbers... you really should define constants for this.

public void getSqrt() {

Anything called "getWhatever" should return some "whatever".

    format(Double.toString(value)); //Sets display to new value

If a method call needs a comment, it's a clear sign that the method was misnamed. What about format -> setDisplay?

} catch(NumberFormatException e){
}

Ignoring exceptions is the deadliest Java sin. Just don't do it.

becoming lazy and skipping quite a lot

public void actionPerformed(ActionEvent ae) {
    if(ae.getSource() == button[0]) numberButtons("7");

    if(ae.getSource() == button[1]) numberButtons("8");

Couldn't button[0] correspond with "0"? This would make some things clearer (and other possible worse). However, you should do something like

  String text = ((Button) ae.getSource()).getText();
  if (text.length() == 1) {
      char c = text.charAt(0);
      if ('0' <= c && c <= '9') {
          numberButtons(c);
      } else {
          operatorButtons(c);
      }
  } else {
      ...
  }

This line

if(ae.getSource() == button[7]) operatorButtons(1); //subtract function[1]

could use a constant and save you a comment. However, calling operatorButtons('-') need neither a constant nor a comment. But as said above, it could look like operatorButtons(c).

becoming even more lazy and stopping for now or forever

\$\endgroup\$
1
  • \$\begingroup\$ This is great and ill get right on it! Thank you so much for your input and seeing examples like these will help me be more efficient. I definitely see where I made some things a little more complicated than they should be. \$\endgroup\$
    – Hobopowers
    Commented May 31, 2015 at 21:30
6
\$\begingroup\$

I'll start with the basics and work my way up. first... If you are using an editor or IDE(IntelliJ, Netbeans, Eclipse) that has the ability to re-format your code, use the format code feature. This keeps things consistent, and a little bit easier to read. Your code wasn't too bad in this regard, but IntelliJ complained about a few things.

Unused methods/variables. Some IDE's can tell you if you are no using a method or a variable. Listen to it. This is especially important for fields. 6 fields are unused: x, parabComplete, frame, x1, x2, and countEquals_quadratic. 2 methods are unused: quadraticFormula, and var. You have 2 unused variables 'spaceFix' on line 312, and c on line 483. (I used the IDE to give me those, so the line numbers might be slightly different on your machine)

Don't compare booleans. 11 times I see where you use == true, or == false. With proper names and some forethought you should be able to make the if statement read a bit more in English. for instance by just renaming parabola to isParabola your mind automatically thinks in terms of booleans.

A quick note about your JButton array button, and your string array buttonString. I would probably restructure the majority of the code, but a quick and easy way to fix what you have it read a bit better would be to possibly to make a hashmap of so you could say if ae.getSource() == button.get("=") that still isn't super nice, but atleast I don't have to count the index of buttonString over and over again.

There are some other nitpicking things I could do that the IDE picked up on, and some are legitimate. but more to your question at hand. How do you make things more OO. Kinda of a loaded question, but in general having a single class for an entire program screams non-oo principles. There are TONS of books, and papers, and blogs about the subject. Reading up on some of them would help tons so I'll only touch on a few things and bother me the most.

SRP (Single Responsibility Principle)

each object should do only 1 thing. It should do it well, and should be tested to show that it can do it well. A byproduct of a object that does one thing well is that it is easy to test. a very simple example would be taking the method getSqrt and making a class that does just that!

public class SquareRootOperation {
    private final String enteredNumber;

    public SquareRootOperation(String enteredNumber) {
        this.enteredNumber = enteredNumber;
    }

    public String performOperation(){
        try {
            double value = Double.parseDouble(enteredNumber);
            if (value == -100)
                return ("John's Girlfriend");
            else {
                value = Math.sqrt(Double.parseDouble(enteredNumber));
                return (Double.toString(value));
            }
        } catch (NumberFormatException e) {
            return enteredNumber + " is not a valid number";
        }
    }
}

NOTE: No Attempt to clean up code was made, just safe encapsulating

now the getSqrt() method looks like this

public void getSqrt() {
    stor = "";
    initial = true;
    format(new SquareRootOperation(display.getText()).performOperation());
    typeDot = false;
    typeNum = false;
}

but even more important I can create a test for SquareRootOperation to make sure that it works as desired.

public class SquareRootOperationTest {

    @Test
    public void testWhenNonNumberIsGivenErrorMessageIsReturned() throws Exception {
        SquareRootOperation operation = new SquareRootOperation("NOT A NUMBER");
        String result = operation.performOperation();
        assertEquals(result, "NOT A NUMBER is not a valid number");
    }

    @Test
    public void testWhenNegative100IsUsedEasterEggShows() throws Exception{
        SquareRootOperation operation = new SquareRootOperation("-100");
        String result = operation.performOperation();
        assertEquals(result, "John's Girlfriend");
    }
}

writing unit tests is getting to be very important. Employeers are looking for people who can write tests and use them. The above can be run in about 400ms. (and even if I add more tests it will still take about that long because the majority of that time was getting the JRE up and running) You may think that writing tests will slow you down, and at first you will be right. However once you get the hang of it and start doing it well you'll ask your self, how can i test this class BEFORE you even started writing it. The whole topic of tests is large, and there are some very vocal proponents of it offering the world. I believe it to be very worth while.

\$\endgroup\$
6
\$\begingroup\$

Too many member variables

There have already been two long reviews so I'll just focus on one thing: you have way too many member variables. You appear to default to declaring all your variables in class scope instead of function scope. You should try to get in the habit of doing it the other way around: make your variables local to a function unless you really need to have them be member variables.

When I took a closer look, I found that many of your variables didn't even have any purpose. Let's examine this block of member variables:

//formatting variables
int var = 0;
double x = 0;
String stor = "";
boolean initial = true;

//variables for Parabola function
int countEquals_parab = 0;
double Angle = 0;
double Vi = 0;
double Vx = 0;
double Vy = 0;
double T_max = 0;
double Y_displ = 0;
double X_displ = 0;
double h = 0;
double k = 0;
double a_parab = 0;
boolean parabComplete = true;
boolean parabola = false;
DecimalFormat df = new DecimalFormat("#######.#####");

My analysis revealed:

  • var: the function var() is never called, so the variable var is never used.
  • x, h, k : unused.
  • initial: not needed, because stor = stor + get will work for both cases.
  • Angle, Vx, Vy, Y_displ, X_displ, a_parab: all could be local variables of parabolaButton().
  • parabComplete: only written, never read, therefore unnecessary.

In other words, 12 out of the 18 member variables in that block of code were either unused/unnecessary or should have been made local variables of the parabolaButton() function.

Later you have this block of member variables:

double a_quadratic = 0;
double b = 0;
double c = 0;

double x1 = 0;
double x2 = 0;
double discr = 0;
int countEquals_quadratic = 0;

All of these variables are unused because the function quadraticFormula() is never called. Even if quadraticFormula() were in use, most of these variables should have been declared as local variables of quadraticFormula() instead.

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

Thanks to everyone who contributed to my problem, I completely scrapped this garbage and made it 1000 times better. I knew from the beginning it was poorly made and I wanted to fix it, I just didn't know where to start. After reading all the advice that was given, watching a few tutorials and brushing up on some simple java concepts (modifiers, jswing, etc), I ended up making a new one that is in MVC format (Yay, order and efficiency). Now all my new variables are actually meaningful (Thanks @maaartinus for helping me realize that many of my variables were poorly named and made my whole program unnecessarily complicated). Also, I tried to work on SRP (Not 100% sure if I completely did it but with the program organized it will be easy to change things) and I plan on adding units later for good practice (Thank you, @Robert Snyder). This new GUI is ugly but that can always be changed later and since It is now in MVC format the job will be easier.

Here is what I did (not finished and far from perfect but a step in the right direction):

CalcGui.java

package com.Calculator;

import java.awt.BorderLayout;
import java.awt.FlowLayout;
import java.awt.event.ActionListener;

import javax.swing.JButton;
import javax.swing.JComboBox;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JOptionPane;
import javax.swing.JPanel;
import javax.swing.JTextField;

public class CalcGui extends JFrame {

private static final long serialVersionUID = 1L;
private String[] operatorsList = { "+", "-", "*", "/", "^" };

// Row 1
private JTextField firstNumber = new JTextField(10);
private JComboBox<String> operator = new JComboBox<>(operatorsList);
private JTextField secondNumber = new JTextField(10);
private JButton calculateButton = new JButton("Calculate");
private JTextField calcSolution = new JTextField(20);

// Row 2
private JLabel sqrtSymbol = new JLabel("√");
private JTextField sqrtNumber = new JTextField(10);
private JButton sqrtCalcButton = new JButton("Calculate");
private JTextField sqrtCalcSolution = new JTextField(20);

// Row 3
private JLabel quadraticLabel1 = new JLabel("A = ");
private JTextField quadraticFirstNumber = new JTextField(5);
private JLabel quadraticLabel2 = new JLabel("B = ");
private JTextField quadraticSecondNumber = new JTextField(5);
private JLabel quadraticLabel3 = new JLabel("C = ");
private JTextField quadraticThirdNumber = new JTextField(5);
private JButton quadraticCalcButton = new JButton("Calculate");
private JLabel quadraticTextBefore = new JLabel("x =");
private JTextField firstQuadraticCalcSolution = new JTextField(3);
private JLabel quadraticTextMiddle = new JLabel("and x =");
private JTextField secondQuadraticCalcSolution = new JTextField(3);

CalcGui() {

    JPanel calcPanel = new JPanel(new BorderLayout());
    FlowLayout Default = new FlowLayout(FlowLayout.LEFT);
    JPanel row1 = new JPanel(Default);
    JPanel row2 = new JPanel(Default);
    JPanel row3 = new JPanel(Default);

    this.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    this.setSize(650, 150);

    row1.add(firstNumber);
    row1.add(operator);
    row1.add(secondNumber);
    row1.add(calculateButton);
    row1.add(calcSolution);

    row2.add(sqrtSymbol);
    row2.add(sqrtNumber);
    row2.add(sqrtCalcButton);
    row2.add(sqrtCalcSolution);

    row3.add(quadraticLabel1);
    row3.add(quadraticFirstNumber);
    row3.add(quadraticLabel2);
    row3.add(quadraticSecondNumber);
    row3.add(quadraticLabel3);
    row3.add(quadraticThirdNumber);
    row3.add(quadraticCalcButton);
    row3.add(quadraticTextBefore);
    row3.add(firstQuadraticCalcSolution);
    row3.add(quadraticTextMiddle);
    row3.add(secondQuadraticCalcSolution);

    calcPanel.add(row1, BorderLayout.NORTH);
    calcPanel.add(row2, BorderLayout.CENTER);
    calcPanel.add(row3, BorderLayout.SOUTH);
    this.add(calcPanel);

}

// basic calculations methods
public double getFirstNumber() {

    return Double.parseDouble(firstNumber.getText());

}

public String getOperator() {

    return (String) operator.getSelectedItem();

}

public double getSecondNumber() {

    return Double.parseDouble(secondNumber.getText());

}

public void setCalcSolution(double solution) {

    calcSolution.setText(Double.toString(solution));

}

void addCalculateListener(ActionListener listenForCalcButton) {

    calculateButton.addActionListener(listenForCalcButton);

}

void displayErrorMessage(String errorMessage) {

    JOptionPane.showMessageDialog(this, errorMessage);

}

// Square root function methods
public double getSqrtNumber() {

    return Double.parseDouble(sqrtNumber.getText());

}

public void setSqrtCalcSolution(double solution) {

    sqrtCalcSolution.setText(Double.toString(solution));

}

void addSqrtCalcListener(ActionListener listenForSqrtCalcButton) {

    sqrtCalcButton.addActionListener(listenForSqrtCalcButton);

}

// Quadratic formula Methods
public double getQuadraticFirstNumber() {

    return Double.parseDouble(quadraticFirstNumber.getText());

}

public double getQuadraticSecondNumber() {

    return Double.parseDouble(quadraticSecondNumber.getText());

}

public double getQuadraticThirdNumber() {

    return Double.parseDouble(quadraticThirdNumber.getText());

}

public void setFirstQuadraticCalcSolution(double solution) {

    firstQuadraticCalcSolution.setText(Double.toString(solution));

}

public void setSecondQuadraticCalcSolution(double solution) {

    secondQuadraticCalcSolution.setText(Double.toString(solution));

}

void addQuadraticCalcListener(ActionListener listenForQuadraticCalcButton) {

    quadraticCalcButton.addActionListener(listenForQuadraticCalcButton);

}
}

CalcModel.java

package com.Calculator;

public class CalcModel {

private double calcValue;

public void calculate(double firstNumber, double secondNumber,
        String operator) {

    if (operator.equals("+")) {

        calcValue = firstNumber + secondNumber;

    }

    if (operator.equals("-")) {

        calcValue = firstNumber - secondNumber;

    }
    if (operator.equals("*")) {

        calcValue = firstNumber * secondNumber;

    }
    if (operator.equals("/")) {

        calcValue = firstNumber / secondNumber;

    }

    if (operator.equals("^")) {

        calcValue = Math.pow(firstNumber, secondNumber);

    }
}

public double getCalcValue() {

    return calcValue;

}

}

SqrtCalcModel.java

package com.Calculator;

public class SqrtCalcModel {

private double sqrtCalcValue;

public void sqrt(double number) {

    sqrtCalcValue = Math.sqrt(number);

}

public double getSqrtCalcValue() {

    return sqrtCalcValue;

}
}

QuadraticCalcModel.java

package com.Calculator;

import javax.swing.JFrame;
import javax.swing.JOptionPane;

public class QuadraticCalcModel {

private double firstQuadraticCalcValue;
private double secondQuadraticCalcValue;

public void quadraticFormula(double a, double b, double c) {

    double discriminant = (b * b) - (4 * a * c);

    if (discriminant >= 0) {

        firstQuadraticCalcValue = (Math.sqrt((b * b) - (4 * a * c)) + (-b))
                / (2 * a);

        secondQuadraticCalcValue = (Math.sqrt((b * b) - (4 * a * c)) - (-b))
                / (2 * a);

    }

    else {

        JFrame parent = new JFrame();
        JOptionPane.showMessageDialog(parent,
                "This function has no real roots.");

    }

}

public double getFirstQuadraticValue() {

    return firstQuadraticCalcValue;

}

public double getSecondQuadraticValue() {

    return secondQuadraticCalcValue;

}

}

CalculatorControler.java

package com.Calculator;

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

public class CalculatorController {

private CalcGui theGui;
private CalcModel theCalcModel;
private SqrtCalcModel theSqrtCalcModel;
private QuadraticCalcModel theQuadraticCalcModel;

public CalculatorController(CalcGui theGui, CalcModel theCalcModel,
        SqrtCalcModel theSqrtCalcModel,
        QuadraticCalcModel theQuadraticCalcModel) {
    this.theGui = theGui;
    this.theCalcModel = theCalcModel;
    this.theSqrtCalcModel = theSqrtCalcModel;
    this.theQuadraticCalcModel = theQuadraticCalcModel;

    this.theGui.addCalculateListener(new CalcListener());
    this.theGui.addSqrtCalcListener(new SqrtCalcListener());
    this.theGui.addQuadraticCalcListener(new QuadraticCalcListener());
}

class CalcListener implements ActionListener {

    public void actionPerformed(ActionEvent e) {

        double firstNumber, secondNumber = 0;
        String operator;

        try {

            firstNumber = theGui.getFirstNumber();
            operator = theGui.getOperator();
            secondNumber = theGui.getSecondNumber();

            theCalcModel.calculate(firstNumber, secondNumber, operator);

            theGui.setCalcSolution(theCalcModel.getCalcValue());

        }

        catch (NumberFormatException ex) {

            System.out.println(ex);

            theGui.displayErrorMessage("You Need to Enter 2 Numbers");

        }
    }
}

class SqrtCalcListener implements ActionListener {

    public void actionPerformed(ActionEvent e) {

        double number = 0;

        try {

            number = theGui.getSqrtNumber();

            theSqrtCalcModel.sqrt(number);

            theGui.setSqrtCalcSolution(theSqrtCalcModel.getSqrtCalcValue());

        }

        catch (NumberFormatException ex) {
            System.out.println(ex);

            theGui.displayErrorMessage("You Need to enter a Number");
        }
    }
}

class QuadraticCalcListener implements ActionListener {

    public void actionPerformed(ActionEvent e) {

        double a, b, c = 0;

        try {

            a = theGui.getQuadraticFirstNumber();
            b = theGui.getQuadraticSecondNumber();
            c = theGui.getQuadraticThirdNumber();

            theQuadraticCalcModel.quadraticFormula(a, b, c);

            theGui.setFirstQuadraticCalcSolution(theQuadraticCalcModel
                    .getFirstQuadraticValue());

            theGui.setSecondQuadraticCalcSolution(theQuadraticCalcModel
                    .getSecondQuadraticValue());

        }

        catch (NumberFormatException ex) {

            System.out.println(ex);

            theGui.displayErrorMessage("You need to enter 3 numbers.");
        }

    }
}

}

MVCCalculator.java

package com.Calculator;

public class MVCCalculator {

public static void main(String[] args) {

    CalcGui theGui = new CalcGui();

    CalcModel theCalcModel = new CalcModel();

    SqrtCalcModel theSqrtCalcModel = new SqrtCalcModel();

    QuadraticCalcModel theQuadraticCalcModel = new QuadraticCalcModel();

    new CalculatorController(theGui, theCalcModel, theSqrtCalcModel,
            theQuadraticCalcModel);

    theGui.setVisible(true);

}
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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