Grundlegender MDAS Java Swing Calculator
Ich habe vor kurzem angefangen, Java zu lernen und habe beschlossen, einen einfachen MDAS-Rechner in Swing zu erstellen. Ich bin nicht ganz neu in der Programmierung, aber ich mache möglicherweise einige häufige Fehler oder schreibe nicht den effizientesten Code.
Ich wollte einen Taschenrechner erstellen, der mehrere Zahlen und Operationen annehmen kann, bevor die Antwort mit MDAS gefunden wird, anstatt die Antwort nach jeder Operation zurückzugeben und für die nächste zu verwenden.
zB 2 * 3 + 4 - 5 / 5 = 9 statt 1
Mein Code besteht aus einer einzelnen Klasse. Da es nicht viel Code gibt, wusste ich nicht, ob es einen guten Grund gibt, ihn in mehrere Klassen aufzuteilen. Ich habe jedoch noch nie so etwas geschrieben. Bitte zögern Sie nicht, mich zu korrigieren.
Repo mit Beispiel GIF und lauffähigem Glas
package calculator;
import java.awt.BorderLayout;
import java.awt.Dimension;
import java.awt.Font;
import java.awt.GridLayout;
import java.awt.event.ActionEvent;
import java.util.ArrayList;
import javax.swing.AbstractAction;
import javax.swing.BorderFactory;
import javax.swing.Box;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JPanel;
public class GUI extends JFrame {
private static final long serialVersionUID = 1L;
private String title = "Basic MDAS Calculator";
private int currentNumber;
private JLabel displayLabel = new JLabel(String.valueOf(currentNumber), JLabel.RIGHT);
private JPanel panel = new JPanel();
private boolean isClear = true;
final String[] ops = new String[] {"+", "-", "x", "/"};
private ArrayList<Integer> numHistory = new ArrayList<Integer>();
private ArrayList<String> opHistory = new ArrayList<String>();
public GUI() {
setPanel();
setFrame();
}
private void setFrame() {
this.setTitle(title);
this.add(panel, BorderLayout.CENTER);
this.setBounds(10,10,300,700);
this.setResizable(false);
this.setVisible(true);
this.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
}
private void setPanel() {
panel.setBorder(BorderFactory.createEmptyBorder(10, 10, 10, 10));
panel.setLayout(new GridLayout(0, 1));
displayLabel.setFont(new Font("Verdana", Font.PLAIN, 42));
panel.add(displayLabel);
panel.add(Box.createRigidArea(new Dimension(0, 0)));
createButtons();
}
private void createButtons() {
// 0-9
for (int i = 0; i < 10; i++) {
final int num = i;
JButton button = new JButton( new AbstractAction(String.valueOf(i)) {
private static final long serialVersionUID = 1L;
@Override
public void actionPerformed(ActionEvent e) {
// If somebody presses "=" and then types a number, start a new equation instead
// of adding that number to the end like usual
if (!isClear) {
currentNumber = 0;
isClear = true;
}
if (currentNumber == 0) {
currentNumber = num;
} else {
currentNumber = currentNumber * 10 + num;
}
displayLabel.setText(String.valueOf(currentNumber));
}
});
panel.add(button);
}
// +, -, x, /
for (String op : ops) {
JButton button = new JButton( new AbstractAction(op) {
private static final long serialVersionUID = 1L;
@Override
public void actionPerformed(ActionEvent e) {
numHistory.add(currentNumber);
currentNumber = 0;
opHistory.add(op);
displayLabel.setText(op);
}
});
panel.add(button);
}
// =
JButton button = new JButton( new AbstractAction("=") {
private static final long serialVersionUID = 1L;
private int i;
@Override
public void actionPerformed(ActionEvent e) {
// Display result
numHistory.add(currentNumber);
while (opHistory.size() > 0) {
if (opHistory.contains("x")) {
i = opHistory.indexOf("x");
numHistory.set(i, numHistory.get(i) * numHistory.get(i+1));
} else if (opHistory.contains("/")) {
i = opHistory.indexOf("/");
numHistory.set(i, numHistory.get(i) / numHistory.get(i+1));
} else if (opHistory.contains("+")) {
i = opHistory.indexOf("+");
numHistory.set(i, numHistory.get(i) + numHistory.get(i+1));
} else if (opHistory.contains("-")) {
i = opHistory.indexOf("-");
numHistory.set(i, numHistory.get(i) - numHistory.get(i+1));
}
opHistory.remove(i);
numHistory.remove(i+1);
}
displayLabel.setText(String.valueOf(numHistory.get(0)));
currentNumber = numHistory.get(0);
numHistory.clear();
if (isClear) {
isClear = false;
}
}
});
panel.add(button);
}
public static void main(String[] args) {
new GUI();
}
}
Ich würde mich über Tipps freuen.
Antworten
package calculator;
Paketnamen sollten die Software dem Autor zuordnen, wie z com.github.razemoon.basicmdasjavacaluclator.
public class GUI extends JFrame {
Für Java-nming-Konventionen verwenden Sie normalerweise UpperCamelCase und Kleinbuchstaben auch für Akronyme wie "Gui", "HtmlWidgetToolkit" oder "HtmlCssParser".
private static final long serialVersionUID = 1L;
Sie benötigen dieses Feld nur, wenn es sehr wahrscheinlich ist, dass die Klasse serialisiert wird ... in diesem Fall höchstwahrscheinlich nicht.
final String[] ops = new String[] {"+", "-", "x", "/"};
Warum ist das so package-private?
Außerdem sind finalArrays nicht so, finalwie Sie denken, die einzelnen Werte können dennoch geändert werden. Sie möchten höchstwahrscheinlich eine Aufzählung ... eigentlich möchten Sie eine Schnittstelle, aber in diesem Beispiel würde eine Aufzählung höchstwahrscheinlich gut genug funktionieren.
private ArrayList<Integer> numHistory = new ArrayList<Integer>();
private ArrayList<String> opHistory = new ArrayList<String>();
Versuchen Sie in diesem Fall immer, die niedrigste gemeinsame Schnittstelle für Deklarationen zu verwenden List.
private void setFrame() {
this.setTitle(title);
this.add(panel, BorderLayout.CENTER);
this.setBounds(10,10,300,700);
this.setResizable(false);
this.setVisible(true);
this.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
}
Warum benutzt du thishier aber nirgendwo anders?
this.setResizable(false);
Warum? Ihr Rahmen ist, soweit ich sehen kann, perfekt anpassbar. Wenn Sie die Größe nicht ändern, stellen Sie nur sicher, dass Ihre Anwendung unter verschiedenen LaFs und Schriftgrößen unbrauchbar wird.
for (int i = 0; i < 10; i++) {
Ich bin ein sehr hartnäckiger Befürworter dafür, dass Sie nur aus einzelnen Buchstaben Variablennamen verwenden dürfen, wenn Sie sich mit Dimensionen befassen.
for (int number = 0; number <= 9; number++) {
// Or
for (int digit = 0; digit <= 9; digit++) {
final int num = i;
Kürzen Sie Variablennamen nicht, nur weil Sie dies können. Die verringerte Eingabe ist die verringerte Lesbarkeit nicht wert.
In Bezug auf die Erstellung von Schaltflächen möchte ich Hilfsmethoden und Klassen erstellen, die das Lesen des Codes erleichtern. In diesem Fall würde ich mich für Lambdas wie folgt entscheiden:
private JButton createButton(String text, Runnable action) {
return new JButton(new AbstractButton(text) {
@Override
public void actionPerformed(ActionEvent e) {
action.run();
}
})
}
// In createButtons:
panel.add(createButton(Integer.toString(number), () -> {
// Code for the number button goes here.
}));
Eine andere Alternative wäre, eine zu erstellen, private class NumberActiondie eine Zahl in ihrem Konstruktor akzeptiert und die zugehörige Aktion ausführt. Dies würde es Ihnen auch ermöglichen, die endgültige Neuerklärung loszuwerden.
private int i;
Das ist ein sehr schlechter Variablenname.
public GUI() {
setPanel();
setFrame();
}
private void setFrame() {
this.setTitle(title);
this.add(panel, BorderLayout.CENTER);
this.setBounds(10,10,300,700);
this.setResizable(false);
this.setVisible(true);
this.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
}
// ...
public static void main(String[] args) {
new GUI();
}
Es wäre besser, die Verantwortlichkeiten hier aufzuteilen. Der Rahmen selbst ist nur dafür verantwortlich, sein eigenes Layout in Gang zu bringen, während die Hauptmethode dafür verantwortlich sein sollte, dass der Rahmen angezeigt wird.
public GUI() {
setPanel();
setFrame();
}
private void setFrame() {
this.setTitle(title);
this.add(panel, BorderLayout.CENTER);
this.setBounds(10,10,300,700);
this.setResizable(false);
}
// ...
public static void main(String[] args) {
GUI gui = new GUI();
gui.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
gui.setVisible(true);
}
Ihre Logik scheint keinerlei Fehlerbehandlung zu enthalten. Ich glaube, dass das zweimalige Drücken einer Bedientaste nacheinander zu einem Fehler führen sollte.
Möglicherweise wäre es besser, den gesamten Ausdruck wie eingegeben auf den Bildschirm zu drucken und dann den Shunting Yard-Algorithmus anzuwenden , um diesen Ausdruck zu verarbeiten.
Ihre Logik führt keine Dezimalstellen aus und behandelt Überläufe auch nicht ordnungsgemäß. Durch Ändern Ihrer Logik können BigDecimalSie beide problemlos handhaben. Beachten Sie, dass Sie BigDecimals mit einem geeigneten Wert erstellen müssen, um MathContextdie richtige Genauigkeit und das richtige Verhalten zu erzielen.
Wenn Sie eine bereits vorhandene Implementierung lesen möchten, kann ich exp4j für eine Bibliothek mit mathematischen Ausdrücken mit Floats, EvalEx für eine Bibliothek BigDecimalund mein eigenes Projekt jMathPaper für einen Taschenrechner mit verschiedenen GUIs (in Bezug auf die Abstraktion) empfehlen .
Sie können die Array-Deklaration verwenden.
Wenn Sie ein Array mit vordefinierten Werten wünschen, die konstant sein können, können Sie das Array anonym deklarieren.
final String[] ops = {"+", "-", "x", "/"};
Verwenden Sie Enums für die Operationen.
Anstatt ein Array von Operationen zu haben, schlage ich vor, dass Sie stattdessen eine Aufzählung erstellen.
public enum Operators {
PLUS("+"), MINUS("-"), MUL("x"), DIV("/");
private final String operator;
Operators(String operator) {
this.operator = operator;
}
public String getOperator() {
return operator;
}
}
Dies bietet Ihnen mehr Vorteile als das Array, da Sie die Duplizierung entfernen können.
//[...]
for (Operators op : Operators.values()) {
JButton button = new JButton( new AbstractAction(op.getOperator()) {
private static final long serialVersionUID = 1L;
@Override
public void actionPerformed(ActionEvent e) {
numHistory.add(currentNumber);
currentNumber = 0;
opHistory.add(op);
displayLabel.setText(String.valueOf(op.getOperator()));
}
});
panel.add(button);
}
//[...]
//[...]
if (opHistory.contains(Operators.MUL)) {
i = opHistory.indexOf(Operators.MUL);
numHistory.set(i, numHistory.get(i) * numHistory.get(i + 1));
} else if (opHistory.contains(Operators.DIV)) {
i = opHistory.indexOf(Operators.DIV);
numHistory.set(i, numHistory.get(i) / numHistory.get(i + 1));
} else if (opHistory.contains(Operators.PLUS)) {
i = opHistory.indexOf(Operators.PLUS);
numHistory.set(i, numHistory.get(i) + numHistory.get(i + 1));
} else if (opHistory.contains(Operators.MINUS)) {
i = opHistory.indexOf(Operators.MINUS);
numHistory.set(i, numHistory.get(i) - numHistory.get(i + 1));
}
//[...]
Meiner Meinung nach erleichtert dies auch die Arbeit mit dem Code und die Umgestaltung.
Überprüfen Sie beim Teilen immer das, divisorbevor Sie die Teilung durchführen.
Wenn durch Null geteilt wird, gibt es einen java.lang.ArithmeticExceptionWurf durch Java; Ich schlage vor, dass Sie einen Scheck hinzufügen :)
Verwenden Sie das Queueanstelle von List, um den Verlauf zu speichern.
Wenn ListSie das verwenden, müssen Sie einen Index verwenden, Queueum das erste Element ( java.util.Queue#poll) zu entfernen . Der einzige Nachteil ist, dass Sie den eigentlichen Code umgestalten müssen, um den zu entfernen indexOf.
private Queue<String> opHistory = new ArrayDeque<>();
Auf diese Weise wird der Code kürzer.
while (opHistory.size() > 0) {
Operators currentOperator = opHistory.poll();
switch (currentOperator) { //Java 14+ Switch, you can use if or the older version of the switch.
case MUL -> numHistory.set(i, numHistory.get(i) * numHistory.get(i+1));
case DIV -> numHistory.set(i, numHistory.get(i) / numHistory.get(i+1));
case PLUS -> numHistory.set(i, numHistory.get(i) + numHistory.get(i+1));
case MINUS -> numHistory.set(i, numHistory.get(i) - numHistory.get(i+1));
}
numHistory.remove(i + 1);
}
Extrahieren Sie den Ausdruck bei mehrfacher Verwendung in Variablen.
In Ihrem Code können Sie ähnliche Ausdrücke in Variablen extrahieren. Dadurch wird der Code kürzer und leichter lesbar.
final Integer first = numHistory.get(i);
final Integer second = numHistory.get(i + 1);
switch (currentOperator) {
case MUL -> numHistory.set(i, first * second);
case DIV -> numHistory.set(i, first / second);
case PLUS -> numHistory.set(i, first + second);
case MINUS -> numHistory.set(i, first - second);
}