すずかのプログラミング勉強記

元教員からエンジニアを目指す、プログラミング勉強記録です。

トップ > Ruby > プログラミング初心者がコードレビューで学んだこと【超初心者編】

プログラミング初心者がコードレビューで学んだこと【超初心者編】

Ruby

はじめに

完全未経験からプログラミングを勉強して、約5ヶ月経ちました。
FJORD BOOT CAMP（フィヨルドブートキャンプ）では、fizzbuzzなど初心者向けの課題に取り組んでいる時から、メンターさんに本気のレビューをしていただきます😊

今回は「超初心者編」として、学習1~2ヶ月目にRubyのコードレビューで学んだことを中心にまとめたいと思います。



スペースを適切に入れる

rubocopを導入するまで、半角スペースを入れるのか入れないかをよく迷いました😖

演算子の前後には、半角スペースを入れる。

+や-など、演算子の前後にはスペースが入るのが一般的です。私が書いていた、ダメな例がこちらです。

if number%15 == 0 # ダメ
if number % 15 == 0 # OK

%と出会うのが初めてだったため、演算子だという認識がなく、前後にスペースを入れていませんでした。「何が演算子なの？」という時は、確認しておくべきでした。
Ruby入門 - 演算子 - とほほのWWW入門

メソッドと引数の間には、半角スペースを入れない。

上とは逆に、不要なスペースを入れてしまった例です。

"Ruby!".delete ("!") # deleteメソッドと引数の間に半角スペースが入っている。
=> "Ruby"

このコードでは、スペースが入っていても動きました。

しかしコードによっては、エラーや意図しない結果になってしまいます。

"Ruby!".delete("!") + ("?") # 正しい書き方
=> "Ruby?"
"Ruby!".delete ("!") + ("?") # deleteの後ろに半角スペースあり
=> "Ruby"

下だと「"Ruby!"から"!"と"?"を削除する」という意味になってしまい、想定通りの結果になりません。

Rubyでメソッド呼び出しのかっこの手前にスペースが入るとエラーになる理由 - Qiita


変数・定数をわかりやすく

「名前重要！」というのは毎日聞く言葉ですが、命名についても当然沢山ご指摘を受けました。

変数名に抽象的すぎる名前をつけない

array = [] # ダメな名前
number = 1

while number <= 20
  array << number
  number += 1
end

arrayのような変数名をつけてしまうと、読み手に伝わる情報がない上に、別の配列を使う必要が出た場合困ります。

下のサイトではこういう名前は、使ってはいけない名前とされていました😅
data とか info とか list とか item とかいう変数名止めろ - Neo's World

マジックナンバーは、定数として定義する

ボウリングのスコア計算プログラムを作っていた時のコードです。

next_frame = [3, 4]

def calculate_spare(next_frame)
  next_frame[0] + 10 # 次のフレーム1回目の得点に10を足す
end

def calculate_strike(next_frame)
  next_frame[0] + next_frame[1] + 10  # 次のフレーム1・2回目の得点に10を足す
end

謎の数字（マジックナンバー）の「10」が何度も出てきてしまっています。読んだ人は「この10は一体なに？」と気になることでしょう😣
STRIKE_POINT = 10という定数を最初に定義することで、読みやすくなりました。


無駄な処理を書かない

うっかり不要な処理が残ったまま提出してしまったことが、何回もありました😅

無駄にデータ型を変換しない

user = { name: 'suzuka', age: 28 }
puts user[:name].to_s # 無駄に文字列に変換
puts user[:age].to_i + 1 # 無駄に数字に変換

長い処理を書いているうちに変数の中身がわからなくなり、余計な変換処理をしてしまっていました💦
変数の中身がどのようなデータ型なのか、常に意識しておくべきでした。

デバック用のコードは、削除しておく

require 'debug' # デバッガを有効にする
number = 0
total = []

while total.size < 10
  total << number
  binding.break # デバック用のブレークポイントを追加
  number += 1
  p total # プリントデバッグで変数の中身をチェック
end

debug.gemに関係するコードやpメソッドなど、デバッグで使用するコードが混じっています。他の人がコードを読む際に邪魔なので、削除しておくべきでした。


メソッドの定義をわかりやすく

メソッドを何のために定義するのか、どのように切り出すのかがわからず、悪戦苦闘しました。

メソッドを定義する目的を考える

このような短すぎるメソッドを沢山定義していました。

def sum_numbers(numbers)
  numbers.sum
end

current_numbers = [ 1, 2, 3 ]
current_sum = sum_numbers(current_numbers)

next_numbers = [ 2, 3, 4 ]
next_sum = sum_numbers(next_numbers)

「同じ処理をするならメソッドにするのがいいね！」という気持ちで切り分けましたが、逆に読みにくくなってしまいました。
メソッド多い分だけ、コードを読む時に複数の場所を行き来する必要が出てきます。

コードの可読性を上げたり修正しやすくしたりするなど、メソッドに切り出す目的を意識して、単純な処理のメソッドを量産しないようにします😢

暗黙の了解を満たさないと使えないメソッドを定義しない

def calculate_price(drink_order, food_order)
  # 値段を計算する処理
end

drink_order = { name: "コーヒー", price: 100 }
food_order = { name: "ケーキ", price: 200 }

total_price =
  unless today.sunday?
    calculate_price(drink_order, food_order)
  else
    drink_order[:price] + food_order[:price] # 日曜日は違う処理
  end

注文の値段を計算をするメソッドcalculate_priceを定義していますが、よく読むと使えるのは月〜土のみです😳

「呼び出すときは、暗黙の了解で日曜日以外にしてね」というメソッドは、使いづらくなってしまいます。メソッド名を工夫する、どんな引数でも処理できるようにするなど、わかりやすいメソッドにするべきでした。


条件式・繰り返しをわかりやすく

処理が複雑になるほど、理解が追いつかなくなり、ぐしゃぐしゃのコードを書いてしまうことがありました💧

ifには色々な書き方がある

例えばこのようなコードがあった場合です。

require 'date'
current_date = 【日付またはnilが入る】

if current_date.nil?
  next_date = Date.today + 1
else
  next_date = current_date + 1
end

next_date =をifの前に持ってくることができます。

next_date = 
  if current_date.nil?
    Date.today + 1
  else
    current_date + 1
  end

三項演算子を使って、１行で書くこともできます。

next_date = current_date.nil? ? Date.today + 1 : current_date + 1

状況に応じて、一番良い書き方を選択しましょう。

一つのループ処理の中に、役割を詰め込みすぎない

eachなど繰り返しの処理を、沢山使うのは良くない気がして、こんなコードを書いていました。

file_names = ["test.rb", "file1.txt"]

file_names.each do |file|
  # ファイルの詳細情報を取得する処理
  # ファイルのサイズの合計を計算する処理
  # ファイル名の文字数の最大値を割り出す処理
end

こういうコードは、3つの処理すべての流れを頭の中に入れなければならず、読むのがとても大変です💥
eachの節約をやりすぎず、処理をわかりやすく書くのに必要な分のループを回すべきでした。

感想

過去の自分に対するレビューを見るのは恥ずかしかったですが、成長を実感できました❣️丁寧なレビューをしてくれるメンターさんに改めて感謝です。
今書いているコードを、数ヶ月後に「酷いコードだ」と言えるよう、引き続き勉強を頑張ります💪

時間ができた時に「Webアプリ編」や（いつの日か）「脱初心者編」も書きたいです。