patternjavaModerate
Ballistics and arithmetic calculator
Viewed 0 times
andballisticscalculatorarithmetic
Problem
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
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
Solution
Just a few tips:
Don't use star imports. let you IDE take care of it.
This is fine, but a separate
Consider using miglayout which can do it without the panels.
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
This should look like
Note the spacing, brackets, and the eliminated magic constant.
I'm throwing a TooManyMagicNumbersException. What about
and
added to the other loop?
Then even more magic numbers... you really should define constants for this.
Anything called "getWhatever" should return some "whatever".
If a method call needs a comment, it's a clear sign that the method was misnamed. What about
Ignoring exceptions is the deadliest Java sin. Just don't do it.
becoming lazy and skipping quite a lot
Couldn't
This line
could use a constant and save you a comment. However, calling
becoming even more lazy and stopping for now or forever
- 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 valueIf 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 likeString 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
Code Snippets
import java.awt.*;public class CalcClass extends JFrame implements ActionListener {JPanel[] row = new JPanel[6];String[] buttonString = {"7", "8", "9", "+", ...}for(int i = 0; i < 5; i++)
function[i] = false;Context
StackExchange Code Review Q#92308, answer score: 10
Revisions (0)
No revisions yet.