レビューをもらおう
前回の記事で一応は全体のコードは仕上がった。しかし、まだまだアラが多い。そういうわけで、自分が所属しているオンラインサロンでコードレビューをしてもらった。けっこうたくさんの指摘があったので、ひとつずつ対応していった。
指摘事項一覧
文法関連
-
Rubyのインデントは半角スペース2つ
-
単純な比較は==を使用する/thenも必要なし
-
変数展開を使ったほうがいい
-
ファイル名がわかりにくい(
transfer_money
->player.rb
へ)
メソッドの移動
-
サイコロを振る/お金をかけるのはplayerの中にいれたほうがいい
(playerの行動であるため) -
check_win_lose
やtransfer_money
メソッドはPlayerのメソッドではないかも
読みやすくする
-
main.rb
はゲームの流れの単位でメソッド化すると全体がみやすくなる - 同じ内容が複数ある箇所はまとめる(
strength_relationship
など) -
roll_dice
の引数をなくす - メッセージを出す場所と処理する場所を別にする
その他改善
-
メソッドの戻り値が文字列なのはよくない
定数または列挙子を使用 -
班長の賭け金の設定がもったいない(所持金に応じて賭け金を変える)
-
演算子で勝敗やお金の移動ができるといい。
多すぎ!!!
指摘事項の対応1:文法関連
やっていこう。まずは文法関連だが、これはわりとすぐに改善できた。インデントの修正はVSCodeの場合は楽にできた。以下の記事を参考にした。
変数展開はこんなこと。こんなふうにするとすっきり書ける。
dice_hand = '通常の目(' + uniq_value.to_s + ')'
↓
dice_hand = "通常の目(#{uniq_value.to_s})"
対応したissueのURL
https://github.com/kyokucho1989/ruby-game/issues/13
指摘事項の対応2: メソッドの移動とか
これはもろもろがんばった。まずは勝敗判定や、賭け金の移動を行うのがPlayerではおかしいとのことだったので、あらたにGameクラスを作成した。そのGameクラスが勝敗判定や賭け金の移動を行う。
また、それを行うのはクラスメソッドでやることにした。gameクラスは状態を持っていないためだ。(これも指摘された)
指摘事項の対応3: 読みやすくする
文章を出力させるのもひとつにまとめた。message.rb
をあらたに作成し、そこから全てのメッセージが出るようにした。こうすることで、文章を修正する時に楽になる。いいね!
指摘事項の対応4: その他
- メソッドの戻り値が文字列なのはよくない(定数または列挙子を使用)
これが難しかった。たしかに文字列で返すと、ちょっとしたタイプミスでバグが生じる。ちょっとがんばることにした。
あらたに module-hand-game.rb
を作成し、役と勝敗結果を定数にした。
module Match
WIN = '勝ち'
LOSE = '負け'
DRAW = '引き分け'
end
参照するときはこんなかんじ。いいね!
if(my_hand_rank > opponent_hand_rank)
Match::WIN
elsif(my_hand_rank == opponent_hand_rank)
Match::DRAW
else
Match::LOSE
end
「演算子で勝敗やお金の移動ができるといい。」は対応できなかった。むずい。
おわりに
Rubyだけでこんなにプログラミングができるのが楽しかった。Railsを含めていろいろやると、環境構築のエラーなどでやる気がそがれがち。はじめはRubyだけでなんか作った方が単純に力がつく気がする。
いつかこれで教材を作りたいなぁ。おわり。
完成したコードはこちらにあります↓
https://github.com/kyokucho1989/ruby-game