Calculadora básica de MDAS Java Swing
Recientemente comencé a aprender Java y decidí hacer una calculadora MDAS básica en Swing. No soy completamente nuevo en la programación, pero puedo estar cometiendo algunos errores comunes o no escribir el código más eficiente.
Quería hacer una calculadora que pueda tomar múltiples números y operaciones antes de encontrar la respuesta usando MDAS, en lugar de simplemente devolver la respuesta después de cada operación y usarla para la siguiente.
por ejemplo, 2 * 3 + 4 - 5 / 5 = 9 en lugar de 1
Mi código consta de una sola clase. No hay mucho código, así que no sabía si había una buena razón para dividirlo en varias clases, sin embargo, nunca había escrito algo como esto, así que no dudes en corregirme.
Repo con gif de ejemplo y jar ejecutable
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();
}
}
Agradecería cualquier consejo.
Respuestas
package calculator;
Los nombres de los paquetes deben asociar el software con el autor, como com.github.razemoon.basicmdasjavacaluclator.
public class GUI extends JFrame {
Para las convenciones de nomenclatura de Java, normalmente usaría UpperCamelCase y usaría minúsculas incluso para acrónimos, como "Gui", "HtmlWidgetToolkit" o "HtmlCssParser".
private static final long serialVersionUID = 1L;
Solo necesita este campo si es muy probable que la clase se serialice ... en este caso, lo más probable es que no.
final String[] ops = new String[] {"+", "-", "x", "/"};
¿Por qué es esto package-private?
Además, las finalmatrices no son finalcomo pensaría, los valores individuales aún se pueden cambiar. Lo más probable es que desee un Enum ... en realidad, desea una interfaz, pero en este ejemplo, un Enum probablemente funcionaría lo suficientemente bien.
private ArrayList<Integer> numHistory = new ArrayList<Integer>();
private ArrayList<String> opHistory = new ArrayList<String>();
Siempre intente utilizar la interfaz común más baja para las declaraciones, en este caso 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);
}
¿Por qué lo usa thisaquí pero en ningún otro lugar?
this.setResizable(false);
¿Por qué? Tu marco es perfectamente redimensionable por lo que puedo ver. Al configurarlo como no redimensionable, solo se asegura de que su aplicación se vuelva inutilizable con diferentes LaF y tamaños de fuente.
for (int i = 0; i < 10; i++) {
Soy un defensor muy persistente de que solo se le permite usar nombres de variables de una sola letra si se trata de dimensiones.
for (int number = 0; number <= 9; number++) {
// Or
for (int digit = 0; digit <= 9; digit++) {
final int num = i;
No acorte los nombres de las variables solo porque puede, la menor cantidad de escritura no vale la menor legibilidad.
En cuanto a la creación de botones, me gusta crear métodos y clases auxiliares que faciliten la lectura del código, en este caso optaría por lambdas, así:
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.
}));
Otra alternativa sería crear un private class NumberActionque acepte un número en su constructor y realice la acción asociada. Eso también le permitiría deshacerse de la redeclaración final.
private int i;
Ese es un nombre de variable muy malo.
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();
}
Sería mejor dividir las responsabilidades aquí. El marco en sí solo es responsable de poner en marcha su propio diseño, mientras que el método principal debería ser responsable de mostrar el marco.
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);
}
Su lógica no parece contener ningún tipo de manejo de errores, creo que presionar un botón de operador dos veces seguidas debería producir un error.
Quizás un mejor enfoque sería imprimir la expresión completa en la pantalla tal como se ingresó, y luego aplicar el algoritmo Shunting Yard para procesar esa expresión.
Su lógica no hace decimales, ni maneja con gracia los desbordamientos. Al cambiar la lógica que usa BigDecimal, puede manejar ambos fácilmente. Tenga en cuenta que debe crear BigDecimalmensajes de correo electrónico con una MathContextprecisión y un comportamiento adecuados.
Si desea leer una implementación ya existente, puedo recomendar exp4j para una biblioteca de expresión matemática usando flotadores, EvalEx para uno que usa BigDecimaly mi propio proyecto jMathPaper para una calculadora que tiene diferentes GUI (con respecto a la abstracción).
Puede utilizar la declaración de matriz.
Cuando desee una matriz con valores predefinidos que puedan ser constantes, puede declarar la matriz de forma anónima.
final String[] ops = {"+", "-", "x", "/"};
Utilice Enums para las operaciones.
En lugar de tener una matriz de operaciones, le sugiero que cree una Enum.
public enum Operators {
PLUS("+"), MINUS("-"), MUL("x"), DIV("/");
private final String operator;
Operators(String operator) {
this.operator = operator;
}
public String getOperator() {
return operator;
}
}
Esto le dará más ventajas que la matriz, ya que podrá eliminar la duplicación.
//[...]
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));
}
//[...]
Además, en mi opinión, esto hará que el código sea más fácil de trabajar y refactorizar.
Al dividir, siempre marque divisorantes de hacer la división.
Al dividir por cero, hay un java.lang.ArithmeticExceptionlanzamiento de java; Sugiero que agregue un cheque :)
Utilice el en Queuelugar de Listpara mantener el historial.
Al usar el List, debe usar un índice, el Queuepara eliminar el primer elemento ( java.util.Queue#poll); El único inconveniente es que deberá refactorizar el código real para eliminar el indexOf.
private Queue<String> opHistory = new ArrayDeque<>();
Al hacerlo, acortará el código.
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);
}
Extrae la expresión a variables cuando se usa varias veces.
En su código, puede extraer las expresiones similares en variables; esto hará que el código sea más corto y más fácil de leer.
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);
}