プログラムに「意味のない行」なんてない

今日は先輩たちにコードレビューをしてもらいました。
勉強になりました。ありがとうございました。

勉強になった点

  1. 数字はあまり使わない(マジックナンバー)
  2. 直感的なコードを書く
  3. コードに意味を持たせる

以上はコードを書く上で、大切なことだと学びました。「Rubyでじゃんけんしましょう」の記事で書いた"定数を使って解りやすいコードを書く"事を活かせてなかった。
レビュー前のコードを見てください。

#数字を読み込む関数
     1  # === 概要
     2  # 
     3  # ユーザに数字を入力して1-6を判定する
     4  # 
     5  # === 戻り値
     6  #
     7  # ユーザが入れた値を返す
     8  def input_number()
　　　　(...省略...)
    15    return usr_num
    16  end
    17      
    18  
    19  
    20  trial_count = 0
    21  usr_count = 0
    22  diff = 0
    23  while trial_count < 3 && usr_count == 0
    24    usr_num = input_number()
    25    computer = rand(6) + 1
    26    diff = computer - usr_num
    27    puts("computer:#{computer}")
    28  
    29    if diff == 0
    30      puts("win")
    31      usr_count += 1
    32    elsif diff == 1 || diff == -1 || diff == 5 || diff == -5
    33        puts("#{diff}")
    34        puts("close!!!!") 
    35    else
    36      puts("lose")
    37    end
    38      
    39  trial_count += 1
    40  end

rand()関数で与えた1〜6までの数をプレーヤが当てるゲームです。
このコードはひどい。
何がひどいかというと、
＊line32の条件式が多い。
＊マジックナンバーが多い(line23　trial_count < 3)
＊読んでいて理解しにくい
これらの悪いところを直すと
＊line32の条件式が多い。→条件式はシンプルに。
＊マジックナンバーが多い→定数を置いてわかりやすく＆変更しやすいように
＊読んでいて理解しにくい→直感的なプログラムを書く

レビューで教えてもらったことを反映して、上記プログラムを書き直してみました。

 1 GAME_ROUND=3
 2 NO_SCORE=0
 3MAX=6
 4 MIN=1
 5 CLOSE=1


 6 # === 概要
 7 # 
 8 # ユーザに数字を入力して1-6を判定する
 9 # 
10 # === 戻り値
11 #
12 # ユーザが入れた値を返す
13 def input_number()
　　　　(...省略...)
14  return usr_num
15 end
    
16 trial_count = 0
17 usr_score = 0
18 diff = 0

19 while trial_count < GAME_ROUND && usr_score == NO_SCORE
20 usr_num = input_number()
21 computer = rand(MAX) + 1
22 puts("#{computer}")

23 if usr_num == computer
24   puts("win")
25   usr_score += 1
26 elsif usr_num - computer == CLOSE
27   puts("close!!")
28 elsif computer - usr_num == CLOSE
29   puts("close!!")
30 else
31   puts("lose")
32 end
   
33 trial_count += 1
34 end

わかりやすくなっているといいなぁ。数字を定数にすることで、意味を持たせ直感的になっています。
ここで"NO_SCORE"と"usr_count"ですが、プログラム内で0と1しか使いません。それなら、「0と1」→「trueとforce」に変えるべきです。
このプログラムでは"usr_score += 1(line9)"となった時点でwhileから抜けるようになっています。whileから抜けたいならbreakで抜ければいい。

ってことで、書き換えます。
・"NO_SCORE = 0"　line2を削除
・"usr_score"　line17を削除
・"while trial_count < GAME_ROUND && usr_score == NO_SCORE"　line19を"while trial_count < GAME_ROUND"に変更
・"usr_score += 1" line25を"break"に変更


breakは教えてもらいました。やっぱり人に見てもらうのって、自分が思いうかばない考えが出てくるから面白い。