中文こんにちは。じゅん(@BoNingennnN)といいます。
普段仕事でRubyを使っています。フレームワークはRailsです。
ゼロイチのフェーズのプロジェクトに関わっていると、スピードをとにかく重視することがあります。
その結果、これまでに私は数々の「臭うコード」を生み出してしまいました。
スピードを重視していたとしても、「臭うコード」を生み出さずクリーンなコードを生み出し続ける優秀なエンジニアもいます。
そう。私はまだまだエンジニアとして未熟なので「臭うコード」を生み出してしまっていたのです。
エンジニアになって一年が経とうとしているので、いつまでも未熟とは言っていられません。
これを機にこれまで私が生み出してきた「臭うコード」を消臭していきたいと思います。
1, evalを使って消臭(リファクタリング)してみる
元々のコードがこちら。(メソッド名や変数名などは変えてあります。)
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 |
class Profile < ApplicationRecord #色ごとの勝利数を変更するメソッド #red_win_num, blue_win_num, white_win_num, green_win_numというカラムが存在するよ def change_win_num(color) if color == "red" self.red_win_num += 1 elsif color == "blue" self.blue_win_num += 1 elsif color == "white" self.white_win_num += 1 elsif color == "green" self.green_win_num += 1 end end end |
このchange_win_numというインスタンスメソッドですが、ほとんど同じ処理を繰り返し書いています。
色(color)によって処理が違うならまだしも、今回の場合やっている処理は「その色のwin_numを+1する」だけで全て共通です。
このように、if文で分岐しているのに繰り返し同じ処理を記述している場合は、evalを使えないか検討してみましょう。(そもそもif文でなくcase ~ whenを使えという声もありました。)
evalの詳しい説明はリファレンスをみていただくとして、簡単に説明すると、evalは第一引数として渡した文字列をRubyプログラムとして実行してくれます。
以下の例をみていただければevalがやっていることが分かると思います。
(文字列 "p message" をevalに渡すとそれを実行してくれるという例)
|
1 2 |
message = "I love Ruby" eval "p message" # => "I love Ruby" |
今回の「臭うコード」をリファクタリングするときに、evalはぴったりそうです。
早速evalを使って先ほどのコードをリファクタリングしていきます。
|
1 2 3 4 5 6 7 |
class Profile < ApplicationRecord def change_win_num(color) eval "self.#{color}_win_num += 1" end end |
めちゃくちゃスッキリしましたね。
colorをいちいちベタがきしていたらそこでバグを埋め込んでしまう可能性もありますが、これなら安心です。万が一他の色が増えたとしても、このメソッドを変更する必要はありません。
（若干の懸念点としては、カラムに存在しないcolorをこのメソッドの引数として渡された場合、エラーが起きてしまいます。）
（また、そもそもProfileに色ごとの勝利数のカラムを持たせていることへの懸念も残ります。このままだと色が増えて行ったらそれだけカラムを増やさないといけなくなってしまいます。それを考慮すると、「profile_id」と「color」と「win_num」の３つのカラムをもつ別テーブルを作った方がよりいい気がします。が今回はevalを使ったリファクタリングのみで一旦やめておきます。）
戻り値が複数のものを1つにしてみよう
Rubyでは、戻り値を複数持つことができます。
この場合、メソッドの呼び出し側でも複数の受け皿(変数)を用意しておく必要があります。
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 |
class Sample < ApplicationRecord #２つの数字を渡すと、足し算、引き算、掛け算、割り算した値を返してくれるクラスメソッド #本来それぞれは別のメソッドにすべきだが、複数の戻り値をもつメソッドの例として無理やり１つのメソッドにしました #ゼロで割られる可能性もあるが、今回はそこは本題じゃないので無視します def self.calculate(num_1, num_2) add_result = num_1 + num_2 #足し算 substract_result = num_1 - num_2 #引き算 multiple_result = num_1 * num_2 #掛け算 divide_result = num_1 / num_2 #割り算 #複数の結果をまとめて返す return add_result, substrac_result, multiple_result, divide_result end end |
呼び出す方はこんな感じで呼び出せます。
|
1 2 3 4 5 6 |
add, substract, multiple, divide = Sample.calculate(30, 10) p add #=> 40 p substract #=> 20 p multiple #=> 300 p divide #=> 3 |
今回の例だと４つの戻り値があります。これがさらに増えると、メソッドの呼び出し元のコードがかなり汚くなります。
呼び出すたびに、その戻り値分の受け皿を用意する必要があるからです。
また、戻り値の順番も気にしないといけなくなるので、これも厄介です。
今回の例だと、1つ目の戻り値が足し算の結果で、2つ目が引き算で、3つ目が掛け算で、4つ目が割り算ということを呼び出し元が認識しておく必要があります。これを間違えると事故が起きる可能性があります。
これらの問題を解消する為に、戻り値を１つにしてみましょう。
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 |
class Sample < ApplicationRecord def self.calculate(num_1, num_2) add_result = num_1 + num_2 #足し算 substract_result = num_1 - num_2 #引き算 multiple_result = num_1 * num_2 #掛け算 divide_result = num_1 / num_2 #割り算 calculated_result = { add: add_result, substract: substract_result, multiple: multiple_result, divide: divide_result } return calculated_result end end |
やっていることは一緒ですが、結果的に戻り値がcalculated_resultという1つのハッシュだけになりました。
呼び出し元を見てみましょう。
|
1 2 3 4 5 6 |
data = Sample.calculate(30, 10) p data[:add] #=> 40 p data[:substract] #=> 20 p data[:multiple] #=> 300 p data[:divide] #=> 3 |
戻り値が１つなので、呼び出す際にかなりスッキリしました。また、戻り値の順番を機にする必要もなくなりました。
これで多少は消臭(リファクタリング)できたかと思います。
今後も随時追加していきます！(週に一個のペースで追加していく予定です)