syunta / reverse-polish-notation Goto Github PK

18fce0c

ファイルを分割するのは構いませんが、元々あった RPN.java が残ったままですね。これでは同じプロジェクトに同じ目的のアプリケーションが2本も入ってる状態になっており、これまでの経緯を知らない人が見た場合にどれを使えばいいのか分からないので、不要になったファイルは削除してください。

18fce0c

入力-計算-出力のループを構成するように、とコメントしましたが、処理の分解の仕方がおかしいですね。「Eclipse で学ぶはじめての Java」の9章 - 特に9.4節を読み直してください。

具体的には各メソッドが名前に反して色んなことをやりすぎていたり、片手落ちの部分があります:

• RPN_input.Input: ユーザーからの入力を取り扱うだけでなく、アプリケーションの終了条件までもが含まれている。
• RPN_keisan.error, RPN_keisan.checkError_1, RPN_keisan.checkError_2: エラーチェックはエラーのあったかどうかだけを調べれば良い。どういうエラーが起きたかをユーザーに知らせることやアプリケーションの制御はエラーチェックに含めて行うようなことではない。
• RPN_main.Flow: これがアプリケーションの心臓部ですが、肝心の終了条件がここにありませんね。

入力処理を別のメソッドに切り分けています。切り分けたこと自体は問題ないのですが、肝心の切り分け方があまり良くありません。

このメソッドは入力を処理するものなので、それ以外の作業は全て余計です。具体的には:

• 入力が尽きた場合にどうするかはこの入力処理を呼んだ側が担当すべき事柄であって、入力処理そのものが担当することではありません。なので「入力が尽きた」ことを示す値を決めて返すべきです。
• 入力が exit だった場合にどうするかも同じです。入力内容をどう処理するかはこのメソッドを呼んだ側が行うべきことであって、入力処理そのものが担当することではありません。なので exit はそのまま返すべきです。

全体的にインデントがはちゃめちゃなところがあります。例えば:

• if の中身のインデントがされてない。
• 4スペースでインデントしているところもあれば8スペースでインデントしているところもあってバラバラ。
• そもそもスペースとタブが入り混じっている

なのでインデントを直してください。Eclipseを使っていれば一括で直せるはずです。

計算処理は大きく分けて「入力内容を数式として解析する」と「解析した数式を評価して結果を求める」の2つになります。ところが今のコードだと両者がいっしょくたになっています。別のメソッドに分離してください。

18fce0c

現状だと Shift JIS になっているようですが、これでは GitHub 上でソースコード中のコメント等が読めないので、 UTF-8 に変更してください。

入力が正しい数式ばかりとは限らないので計算処理で色々なエラーチェックを行っています。これ自体は良いことなのですが、チェック後の処理が余計なことをやり過ぎています。

このメソッドは計算処理ですから、与えられた数式を計算してその結果を返すことだけをすべきです。「数式に誤りがあった場合にどうするか」は計算処理を呼んだ側が考えることであって、計算処理そのものが担当することではありません。なので、戻り値は

• 計算が成功したか失敗したか
• 計算結果(計算が成功した場合のみ)
• 失敗理由(計算が失敗した場合のみ)

の3要素が必要になります。

今のコードだと計算が失敗した場合に

• 失敗理由を表示する。
• 新たな数式の入力を待つ。

ということを行っていますが、これは計算処理そのものとは関係がありません。例えば「計算が失敗したらプログラムを終了する」という仕様に変更しようと思った時、今のコードだと計算処理の奥深くのエラーチェック処理を調べなければならず、全体の構成としてとても不自然です。