✉️
【リファクタリング】コードレビューしてもらって学んだこと
✒️記事を書いた経緯
エンジニアの友人に、コードレビューをしていただきました!
以下、アドバイスしていただいたことをまとめます。
✅リファクタリング
プログラムの動作を変えず、内部構造を修正して、保守や可読性をよくする。
✅コメントアウトの使い方
今までは「何を書いてるか」を書いていたが、正しくは「なぜそうしないか」を書く。
「通常XXになりますが、今回は○○のため、この書き方をしています」…という感じ。
変数名がどれがなんだかわからなくなってしまうので、「何を書いたか」を書いてしまった。
現在の会社では、optionなど汎用性の高いものを使用しているため、その影響だと思う。
通常は、パッと見て何を書いたかわかるような名前にするので、「何を書いたか」は不要。
以下は抽象的な変数を使っている理由。(推測ですが…)
- プロジェクトを「1から開発している」のではなく、過去のコードやライブラリ、モジュールを再利用している
- Perlという古い言語を使用しているため、変数名の命名に対する意識が低い?
- スピード重視の開発のため、一つずつ変数名を考えない。確かに数が多く、一個の案件にあまり時間をかけていない。
✅コードの改行すべきところ、しないところ
間違った書き方
class Character
def hogehoge
puts "こんにちは"
end
end
-
end
とend
の間空けない! -
class
とdef
の間空けない! - 一番最後の
end
の下は一行空ける。それ以上は空けない。
正しい書き方
Class Character
def hogehoge
puts "こんにちは"
end
end
✅単一責任の原則
SOLID原則 参考文献・オブジェクトの広場様
Sが「Single Responsibility Principle」
1つのクラスは1つだけ責任を持つことができる。
最初のコード
- クラスが防御の情報を保持している⇒防御はその時々によって変わるので、保持する必要がない。
- キャラクターの攻撃・敵の攻撃と、コードが長いので、まとめたい。
class Character
def calculate_damage(attack_power)
defense_value = defend
damage = [attack_power - defense_value, 0].max
take_damage(damage)
damage
end
loop do
# キャラクターの攻撃
attack_power = character.attack
enemy.calculate_damage(attack_power)
if judge(character)
break
end
# 敵の攻撃
attack_power = enemy.attack
character.calculate_damage(attack_power)
if judge(enemy)
break
end
end
修正したコード
- キャラクタークラスは、ダメージ計算メソッドだけ保持。
(ダメージ計算による受けた攻撃は、キャラクターが保持する必要があるため) - execute_turn(ターン実行)メソッドをmain.rbに書いて、引数をattackerとdefenderとする。これでキャラクターの攻撃・敵の攻撃と、長かったコードを解消!
- judgeメソッドでは何をjudgeしてるかわかりづらい⇒
judge_defeat
(敗北者)に変更。
class Character
def calculate_damage(attack_value, defense_value)
[attack_value - defense_value, 0].max
end
end
def judge_defeat(target)
if target.hp <= 0
puts "#{target.name} は倒れた!"
return true
end
false
end
def execute_turn(attacker, defender)
attack_value = attacker.attack
defense_value = defender.defend
damage = attacker.calculate_damage(attack_value, defense_value)
defender.take_damage(damage)
judge_defeat(defender)
end
loop do
break if execute_turn(character, enemy)
break if execute_turn(enemy, character)
end
出力結果
PS C:\Users\xxx\Desktop\ruby_test> ruby main.rb
hero は攻撃した!
devil は防御した!
devil は 0 のダメージを受けた!残りHP: 100
devil の残りHPは 100 です。
devil は攻撃した!
……省略……
hero は 4 のダメージを受けた!残りHP: 0
hero の残りHPは 0 です。
hero は倒れた!
Discussion