基本的なMDASJava Swing Calculator

Oct 28 2020

私は最近Javaの学習を開始し、Swingで基本的なMDAS計算機を作成することにしました。私はプログラミングに完全に慣れているわけではありませんが、よくある間違いを犯したり、最も効率的なコードを書いていなかったりする可能性があります。

すべての操作の後に答えを返し、次の操作に使用するのではなく、MDASを使用して答えを見つける前に複数の数値と操作を実行できる計算機を作りたかったのです。

例: 1ではなく2 * 3 + 4 - 5 / 5 = 9

私のコードは単一のクラスで構成されています。コードが少ないので、複数のクラスに分割する理由があるかどうかわかりませんでしたが、こんなものを書いたことがないので、お気軽に訂正してください。

サンプルgifと実行可能なjarを使用したレポ


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();
    }

}

ヒントをいただければ幸いです。

回答

5 Bobby Oct 28 2020 at 23:41
package calculator;

パッケージ名は、のようにソフトウェアを作成者に関連付ける必要がありcom.github.razemoon.basicmdasjavacaluclatorます。


public class GUI extends JFrame {

Java nmingの規則では、通常はUpperCamelCaseを使用し、「Gui」、「HtmlWidgetToolkit」、「HtmlCssParser」などの頭字語でも小文字を使用します。


private static final long serialVersionUID = 1L;

このフィールドが必要になるのは、クラスがシリアル化される可能性が高い場合のみです...この場合、ほとんどの場合、そうではありません。


final String[] ops = new String[] {"+", "-", "x", "/"};

これはなぜpackage-privateですか?

また、final配列はfinal思ったほどではありませんが、個々の値は変更できます。ほとんどの場合、列挙型が必要です。実際には、インターフェイスが必要ですが、この例では、列挙型で十分に機能する可能性があります。


    private ArrayList<Integer> numHistory = new ArrayList<Integer>();
    private ArrayList<String> opHistory = new ArrayList<String>();

この場合、宣言には常に最も低い共通インターフェースを使用するようにしてください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);
    }

なぜthisここで使用しているのに他の場所では使用していないのですか?


        this.setResizable(false);

どうして?私が見る限り、あなたのフレームは完全にサイズ変更可能です。サイズ変更不可に設定することにより、異なるLaFとフォントサイズでアプリケーションが使用できなくなることを確認するだけです。


for (int i = 0; i < 10; i++) {

私は、ディメンションを扱う場合にのみ1文字の変数名を使用できることを強く主張しています。

for (int number = 0; number <= 9; number++) {
// Or
for (int digit = 0; digit <= 9; digit++) {

final int num = i;

できるという理由だけで変数名を短くしないでください。入力の量を減らしても、読みやすさを減らす価値はありません。


ボタンの作成に関しては、コードを読みやすくするヘルパーメソッドとクラスを作成するのが好きです。この場合、次のようなラムダを使用します。

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.
}));

もう1つの方法はprivate class NumberAction、コンストラクターで数値を受け入れ、関連するアクションを実行するを作成することです。それはまたあなたが最終的な再宣言を取り除くことを可能にするでしょう。


private int i;

それは非常に悪い変数名です。


    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();
    }

ここで責任を分担したほうがよいでしょう。フレーム自体は独自のレイアウトを実行することのみを担当し、mainメソッドはフレームを表示することを担当する必要があります。

    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);
    }

あなたのロジックにはエラー処理が含まれていないようです。オペレーターボタンを2回続けて押すと、エラーが発生するはずです。


おそらく、より良いアプローチは、入力されたとおりに式全体を画面に印刷してから、Shunting YardAlgorithmを適用してその式を処理することです。


ロジックは小数を処理せず、オーバーフローを適切に処理しません。使用するロジックを変更することで、BigDecimal両方を簡単に処理できます。適切な精度と動作を得るにはBigDecimal、適切なsを作成する必要があることに注意してくださいMathContext


すでに既存の実装を読みたい場合は、私がお勧めすることができexp4j数学表現、山車を使用してライブラリのEvalExを使用して1のためBigDecimal、そして自分自身のプロジェクトjMathPaperスポーツ異なるのGUI(抽象化に関する)電卓のため。

4 Doi9t Oct 29 2020 at 04:33

配列宣言を使用できます。

定数にできる事前定義された値を持つ配列が必要な場合は、その配列を匿名で宣言できます。

  final String[] ops = {"+", "-", "x", "/"};

操作には列挙型を使用します。

操作の配列を使用する代わりに、代わりに列挙型を作成することをお勧めします。

public enum Operators {
   PLUS("+"), MINUS("-"), MUL("x"), DIV("/");
   private final String operator;
   Operators(String operator) {
      this.operator = operator;
   }
   public String getOperator() {
      return operator;
   }
}

これにより、重複を削除できるため、アレイよりも多くの利点が得られます。

//[...]
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));
}
//[...]

また、私の意見では、これによりコードの操作とリファクタリングが容易になります。

分割するときは、分割するdivisor前に必ず確認してください。

ゼロ除算の場合、java.lang.ArithmeticExceptionJavaによるスローがあります。チェックを追加することをお勧めします:)

Queue代わりにを使用Listして、履歴を保持します。

Listを使用して、インデックスを使用する必要があります。Queue最初のアイテムを削除するには(java.util.Queue#poll); 唯一の欠点は、を削除するために実際のコードをリファクタリングする必要があることindexOfです。

private Queue<String> opHistory = new ArrayDeque<>();

そうすることで、コードを短くすることができます。

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);
}

複数回使用する場合は、式を変数に抽出します。

コードでは、同様の式を変数に抽出できます。これにより、コードが短くなり、読みやすくなります。

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);
}