hiyoko-programingの日記

プログラミングを勉強したてのひよっ子。   エンジニア目指して勉強中。

リファクタリング

リファクタリングとは

リファクタリングとは、処理の内容を変えずに冗長なコードを削除したり、コードを改善すること。

リファクタリングのメリット

リファクタリングを行うことで下記のメリットが得られる。

  • 読み手が最短時間でコードを理解できるようになる
  • 修正がしやすくなる

リファクタリングすることで、効率的に開発していくことができる。

リファクタリングを行うためには

リファクタリングを行うための要素は以下。

  • 制御フロー
  • 式の分割
  • 変数
  • メソッドの抽象化
  • 関心の分離
  • 短いコードを書く

制御フロー

条件分岐や繰り返し処理などの制御フローがあると他の場所に処理が飛んだり、枝分かれしたりとコードがわかりにくくなりがちである。これから制御フローがあるコードを読みやすく書けるようにしたい。

制御フローのポイント

制御フローのポイントは以下のようになる。

  • 条件式の並び順
  • if/elseの順番

条件式の並び順

条件式の引数の並び順も気をつける必要がある。左側には、値が変化する「調査対象」の式を右側には、値が変化しない「比較対象」の式を書くと読みやすいコードを書くことができる。

1
2
3
4
5
6
7
8
9
#bad
if  10 <= number 
  省略
end

#good
if number >= 10 
  省略
end

if/elseの順番

if/elseのブロックはtrueの場合を先に書くこともif/elseの場合を先に書くこともできるように並び順を自由に変えることができる。今後は条件式を書く際は、以下の3つの注意点に気をつけることで、読みやすいコードを書くことができる。

  • 条件式は肯定形を使う
  • 単純な条件を先に書く
  • 関心を引く条件を先に書く

-①条件式は肯定形を使う

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
 #bad
 #否定文から条件を定義する
 if  a != b 
   #条件2
 else
   #条件2
 end

 #good
 #肯定文から条件を定義する
 if  a == b 
   #条件1
 else
   #条件2
 end

②単純な条件を先に書く
読み手が、できるだけ頭を使わずコードを理解できるように条件を並び替える。

例 
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
#bad
if  a && b == 0
  if c
    # 処理
  end
end

#good
if c
  if a && b == 0
    #処理
  end
end

③関心を引く条件を先に書く
否定の条件であっても、単純で関心や注意を引く場合がある。そういうときは、否定の条件でも先に処理を行う。

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
#bad
if user.image
  # 処理
else
  #エラーを表示する
end

#good
if user.image.nil
  # エラーを表示する
if user.image
  # 処理
end   

関数から早く返す

関数で複数のreturn文を使ってはいけないと思うかもしれないが、関数からfalseの場合の処理を早く返すことで、安全にtrueの時の処理を実行できる。

1
2
3
4
def check_bad_word(word) {
  return unless word
  //以降なんらかの処理
}

ネストを浅くする

ネストが深いコードは、読みにくいコードである。なぜなら、読み手は条件を覚えておく必要があり、ネストが深いほど条件を見直しにコードを戻らなければならないからである。

以下のように「本のreviewimageの情報」を登録する場合、imageに値がなくてもtextに値が入っていれば登録し、textに値が入っていない場合は登録できないアプリがあったとする。
ここで、if文の入れ子を深くしてしまうと条件を忘れてしまい、再度条件を見直さなくてはいけなくなってしまう。なので、なるべくネストしないコードを書くようにする。

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
# bad
if @book.review
  if @book.image.nil?
    @book.save
  end
else
  puts 'Please fill in the review'
end

# good
if @book.review
  @book.save
else
  puts 'Please fill in the blank'
end

式の分割

1つの処理のコードが大きくなれば、理解するのが難しくなるだけでなくメンテナンスも難しくなる。読み手に理解してもらうためには、コードを分割する必要がある。コードを分割する方法として、式を表す変数説明変数を用いること。

説明変数のメリット

式の値を保持する説明変数には3つの利点がある。

  • 大きなコードの塊を分割できる。
  • 簡単な名前で式を説明することで、コードを文書化できる。
  • コードの主要な概念を読み手が認識しやすくなる。
1
2
3
4
5
6
7
8
9
if @tweets.user.nickname.nil?
  # 処理
end

#説明変数を使った場合
is_nickname = currnent_user.nickname.nil?
if  is_nickname
  #処理
end

変数

変数を適当に使うとコードは理解しにくくなる。変数名が適当なことで起こる問題は、以下の3つ。

  • 変数が多いと変数を追跡するのが難しくなる
  • 変数のスコープが大きいとスコープを把握する時間が長くなる
  • 変数が頻繁に変更されると現在の値を把握するのが難しくなる

変数の問題への対処法

変数の問題への対処法は以下の3つ。

  • 邪魔な変数を削除する
  • 変数のスコープをできるだけ小さくする
  • 変数は1度だけ使う

不要な変数を削除する

不要な変数を削除すれば、コードは簡潔で理解しやすくなる。以下、gemのdeviseのcurrent_userを使った例になっています。

1
2
3
4
5
6
#bad
user_id = current_user.id
tweet.user.id = user_id

#good
tweet.user.id = current_user.id

説明変数としてcurrent_user.idをuser_idに代入している。しかし、user_idcurrent_user.idを代入する必要はない。

理由は以下の3点です。

  • 複雑な式を分割していない
  • current_user.idのままでも十分明確
  • 変数を一度しか使っていないので、重複したコードの削除になっていない

変数のスコープをできるだけ小さくする

どこからでも参照・更新が可能であるグローバル変数(先頭に$が付いた変数)はなるべく使わない方が良い。また、変数のスコープが大きいと変数を追うために読むコードの量が多くなってしまうので、コードを読みやすくするために変数が見えるコードの行数をできるだけ短くする。

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
# bad
class bookStore 
  $book = "楽しいRuby"
  def stack_book
    $book = ...;
    buy_book
  end

  def buy_book
    //$bookを使っている
  end
end

# good
class bookStore 
  def read_book
   book = "楽しいRuby"
    buy_book(book)
  end

  def buy_book(book)
    //bookを使っている
  end
end

どちらも例のコードも動作は同じだが、goodの例の方はbook呼び出し時にbookの再定義をしなくてよくなる。

変数は1度だけ使う

変数は1度定義していても、他の値を再代入することが可能。しかし、変数が絶えず変更され続けるとコードは理解しにくくなる。永続的に変更されない変数は扱いやすいので、何度も変数に代入しないようにする。

メソッドの抽象化

定義した関数の目的を明確にし、直接的に関係ないものは別の関数にしてまとめるようにする。汎用的なコードをメソッド化することで、あとで使える場面もよくある。

以下の例では、「ユーザーに本を選ばせる」、「ユーザーが選んだ本を表示させる」という2つの処理が行われている。show_bookメソッドでは、「ユーザーが選んだ本を表示させる」ようにしたいので、「ユーザーに本を選ばせる」という処理は別の関数にまとめた方が読みやすくなる。また、「ユーザーに本を選ばせる」という処理は「本を削除する」関数でも使うことができそう。このように汎用的な関数を分けることで、幅広く応用することができる。

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
#bad
def show_book
  index = 0
  @books.each_with_index { |val, index|
    puts "#{index + 1}#{book.title}"
   }
  book = @books[select]
  puts "#{book.name}"
  puts "#{book.image}" 
end

#good
def chose_book
  index = 0
  @books.each do |book|
    puts "#{index} : #{book.name}"
  end
  select = gets.to_i
  show_book(select)
end

def show_book(select)
  book = @books[select]
  puts "#{book.name}"
  puts "#{book.image}" 
end

関心の分離

「関心の分離」といってもイメージしづらいと思います。ここでの「関心」は、「機能」に置き換えて考えると分かりやすいです。 どこからどこまでが1つの機能なのかを整理し分けてコードを書いていくということになります。1度に複数の処理をするコードは読みやすいコードとは言えません。関数の中で処理することは1つの機能だけにしてあげましょう。

1
  2
  3
  4
  5
  6
  7
  8
  9
 10
 11
 12
 13
 14
 15
 16
 17
 18
 19
 20
 21
 22
 23
 24
 25
 26
 27
 28
 29
 30
 31
 32
 33
 34
 35
 36
 37
 38
 39
 40
 41
 42
 43
 44
 45
 46
 47
 48
 49
 50
 51
 52
 53
 54
 55
 56
 57
 58
 59
 60
 61
 62
 63
 64
 65
 66
 67
 68
 69
 70
 71
 72
 73
 74
 75
 76
 77
 78
 79
 80
 81
 82
 83
 84
 85
 86
 87
 88
 89
 90
 91
 92
 93
 94
 95
 96
 97
 98
 99
100
#bad
def order_drink
  orders = []
  order = {}
  puts "ドリンクの名前を入力してください"
  order[:name] = gets.chomp
  puts "注文数を入力してください"
  order[:stock] = gets.to_i

  orders << order

  puts '[0]注文を確認する\n[1]注文を取り消す'
  input = gets.to_i


  if input == 0

    sum = 0
    orders.each do |order|
      puts "#{order[:name]}#{order[:stock]} 杯"
      sum += order[:stock]
    end


  elsif input == 1
    i = 0
    puts "取り消したい注文の番号を入力してください"
    orders.each do |order|
      puts "[#{i}]:#{order[:name]}"
      i = i + 1
    end
    num = gets.to_i
      orders.delete_at(num)
      puts "削除が完了しました" 
  end
end

post_order


#good
def post_order(orders)
  order = {}
  puts "ドリンクの名前を入力してください"
  order[:name] = gets.chomp
  puts "注文数を入力してください"
  order[:stock] = gets.to_i

  orders << order
end

def index_order(orders)
  sum = 0
  line = "-------------------------------"

    puts line
    orders.each do |order|
    puts "#{order[:name]}#{order[:stock]} 杯"
    sum += order[:stock]
  end
    puts line

end

def destroy(orders)
  i = 0

  line = "-------------------------------"
  puts "取り消したい注文の番号を入力してください"
  orders.each do |order|
    puts line
    puts "[#{i}]:#{order[:name]}"
    i = i + 1
    puts line

  end
  num = gets.to_i
  orders.delete_at(num)
  puts "削除が完了しました"
end

orders = []

while true do
  puts "[1]注文を受ける\n[2]注文一覧を見る\n[3]注文を取り消す\n[4]終了する"
  num = gets.to_i

  if num == 1
    post_order(orders)
  elsif num == 2
    index_order(orders)
  elsif num == 3
    destroy(orders)
  elsif num == 4
    puts "ご利用ありがとうございました"
    exit
  else
    puts "無効な数字です。"
  end
end

短いコードを書く

コードが長いとその分コードを理解する時間も長くなってしまう。できるだけ短くコードを書くように注意する。コードが長くなりそうなときは、ライブラリを使用するなどしてコードを短くする方法を考える。

短いコードを書くポイント

短いコードを書くために気をつけていくべきことは下記の4つです。

  • 汎用的なコードを作り、重複コードを削除する
  • 未使用コードや無用の機能は削除する
  • 最も簡単に問題を解決できるような要求を考える
  • 定期的にすべてのAPIを読んで、標準ライブラリに慣れ親しんでおく

汎用的なコードを作り、重複コードを削除する

定義した関数に直接的に関係ないものは別の関数にしてまとめるようにする。汎用的なコードをメソッド化することで、重複しているコードを削除するので読み手が理解しやすいコードを書くことができる。

未使用コードや無用の機能は削除する

新しいプロジェクトをはじめるとき、機能を過剰に見積もり過ぎてしまうことが多々ある。しかし、その結果機能が完成しなかったり全く使われない機能がでてくる。アプリケーションを複雑にしてしまうので、未使用コードや無用のコードは削除してしまう。

最も簡単に問題を解決できるような要求を考える

すべてのプログラムを100%正しく処理させる必要はない。最も簡単に問題を解決できるような要求にするために必要な要求だけに絞って解決する。

例えば、店舗検索システムがあるとします。要求は以下です。
任意のユーザーの緯度経度に対して、最も近い店舗を検索する

これを100%正しく実装するためには下記のことを考慮しなくてはならない。

  • 日付変更線をまたいでいるときの処理
  • 北極や南極に近いときの処理
  • 「1マイルあたりの緯度」に対応した地球の曲率の調整

この条件を真面目に処理すれば、相当な量のコードを書かなくてはならない。
ユーザーが指定した条件の店舗は、少ないかもしれない。これに対して上記の処理を行う必要はない。ユーザーからの要求を削除する。テキサス州のユーザーのために、テキサスで最も近くにある店舗を検索するに変更する。
これなら、すべての店舗との距離を計算すれば良いので簡単に解決ができる。

定期的にすべてのAPIを読んで、標準ライブラリに慣れ親しんでおく

既存のライブラリで問題を解決できることを知っているか知らないかでコードが全く違ってくる。ライブラリ機能を熟知し積極的に活用する。

定期的に標準ライブラリのすべての関数・モジュール・型の名前を
目を通しておけば、新しいコードを書いているときに「ライブラリであったような...」と思い出すことができる。