ゲームプログラマのための設計シリーズ：原理・原則編の記事です。
概要
- DRYは、コードではなく知識の重複を避けること
- 共通化した部分にオプションが増えてきたら要点検
- 関数を「どのように使われるか」ではなく「何をするか」で命名すると、誤ったDRYに気付けることがある
本文
DRYとは
Don't Repeat Yourselfの略で、「繰り返しを避けよ」の意です。
ゲームプログラマの忙しい日々
さて、とあるプログラマＰ君はゲームデザイナーのＧさんから
①「敵Aは、右パンチ→左パンチ→キックの順番で攻撃して、最後に転んじゃうんだ」
②「敵Bは手ごわいから、転ぶ代わりにビームを撃ってくるようにしたいな」
と注文を受けたとします。
void EnemyA_Action()
{
PunchR();
PunchL();
Kick();
Tumble(); // 転ぶ
}
void EnemyB_Action()
{
PunchR();
PunchL();
Kick();
Beam(); // ビーム
}
DRY原則を思い出したP君は、共通部分を関数に切り出すことにしました。
// 敵の行動の共通部分
void Enemy_CommonAction()
{
PunchR();
PunchL();
Kick();
}
void EnemyA_Action()
{
Enemy_CommonAction();
Tumble();
}
void EnemyB_Action()
{
Enemy_CommonAction();
Beam();
}
繰り返しがなくなっていい感じです。
次の日、Ｇさんに
「敵A、Bともに、パンチの前に吠えるようにしてほしいな」
と言われました。
void Enemy_CommonAction()
{
Bark(); // 吠える
PunchR();
PunchL();
Kick();
}
共通化したおかげで、一か所修正するだけで仕事が終わりました！
やはりDRY原則は素晴らしい・・・
さらに次の日、Ｇさんに
「敵Bは、吠えた後パンチの前にビームを出してほしいんだけど・・・」
と相談されました。
P君はちょっと悩みましたが、フラグ引数による分岐を足して対処することにしました。
// isBeamがtrueなら、吠えた後にビームを放ちます
void Enemy_CommonAction(bool isBeam)
{
Bark();
if (isBeam)
{
Beam(); // ビームを打つ
}
PunchR();
PunchL();
Kick();
}
EnemyAとEnemyBのコードから引数を渡すように修正して、今日の作業を終えました。
この日はなんだか少し寝つきが悪かったようです。
今日はプロデューサーさんがROMをチェックする特別な日。
別のゲームデザイナーHさんから
「急ぎ敵Ｃを実装してほしい！挙動は基本今の敵Ａと同じなんだけど、最後に大爆発してプレイヤーを巻き込もうとするんだ。」
と緊迫した表情で言われました。
ここは目の前でサクッと済ませてしまいましょう。
void EnemyC_Action()
{
EnemyA_Action(); // 基本は敵Aと同じ
Blast(); // 最後に大爆発
}
「いいね！これでプロデューサーに見てもらうよ！ありがとう！」Hさんは大急ぎで走り去っていきました。
今度は、Gさんが息を切らしてやってきました。
「急なお願いですまない！敵Aも敵Bも、キックのあと後退するようにしてほしい！ただ、敵Aはその前にまた転んじゃうんだ」
敵AとBの共通部はEnemy_CommonActionに切り出してあるので、そこに追加することにしました。
// isBeamがtrueなら吠えた後ビーム
// isTumbleがtrueならキックのあと転倒
void Enemy_CommonAction(bool isBeam, bool isTumble)
{
Bark();
if (isBeam)
{
Beam();
}
PunchR();
PunchL();
Kick();
if (isTumble)
{
Tumble(); // 転倒する
}
Back(); // 後退する
}
void EnemyA_Action()
{
Enemy_CommonAction(false, true);
Tumble();
}
void EnemyB_Action()
{
Enemy_CommonAction(true, false);
Beam();
}
void EnemyC_Action()
{
EnemyA_Action();
Blast();
}
ここまでの修正を取り込んでROMを作ることになりました。
やれやれ・・・
夕方プロデューサーがやってきて、敵の挙動を確認していきます。
敵A,敵Bの確認が終わり、ここまで満足そうです。
敵Cの確認が始まりましたが・・・おっと、大爆発でプレイヤーを巻き込むはずが、その前に後退してプレイヤーから遠ざかってしまいました。P君は青ざめました・・・Hさんに見てもらったときは大丈夫だったのに、エンバグです！
急いで原因を調べますが、コードもなんだか追いづらくなってしまいました。修正を入れたらもっと汚くなってしまいました。
// isBeamがtrueなら吠えた後ビーム
// isTumbleがtrueならキックのあと転倒
// isBackがtrueなら最後に後退
void Enemy_CommonAction(bool isBeam, bool isTumble, bool isBack)
{
Bark();
if (isBeam)
{
Beam();
}
PunchR();
PunchL();
Kick();
if (isTumble)
{
Tumble();
}
if (isBack)
{
Back();
}
}
// EnemyAとEnemyCの共通部
void EnemyA_Action_Impl(bool isTumble, bool isBack)
{
Enemy_CommonAction(isTumble, true, true);
Tumble();
}
void EnemyA_Action()
{
EnemyA_Action_Impl(true, true);
}
void EnemyB_Action()
{
Enemy_CommonAction(true, false, true);
Beam();
}
void EnemyC_Action()
{
EnemyA_Action_Impl(false, false);
Blast();
}
コード量の削減のためだけにDRYするのは危険
何がいけなかったのでしょうか？
いろいろツッコミどころはありますが、大きな原因に
結果的に敵A,B,Cで共通部位があったのはたまたまだったのに、共通化してしまった という点が挙げられます。
「結果的に」と言っているのは、敵A,B,Cで挙動を共通化すべきかどうかは最初に発注された段階ではわからないからです。今回の例のように、最初は共通化した方がよさそうに見えても、だんだんと共通部分がなくなっていくことは多いので実際の判断はなかなか難しいです。
- 最初に発注を受けた段階では共通化せず、仕様が固まってきてから共通化する
- リビジョン4のように共通化部分にフラグ引数が入ってきた時点で見直す
- バグが出たら見直す（製品版までにバグが取れていればよしとする）
どれもそれぞれ正解かなと思います。
共通化した関数の名前に注目
敵Aと敵Bの共通化に使った関数名はEnemy_CommonActionですが、これは「この関数は何をするか」ではなく「どんな時に使うのか」という命名になっています。実はこれはよくない命名で、共通化すべき処理でないものを共通化してしまっているときに顕著な傾向です。
さて、今回のゲームには「パンチを二発入れると相手のガードが崩せる」という仕様があったとします。すると、
PunchR();
PunchL();
この並びには「相手のガードを崩す処理である」という意味が生まれます。
// ガードを崩す
void OpenUp()
{
PunchR();
PunchL();
}
今度は「この関数は何をするのか」という命名になりました。
過度な共通化もなくして素朴な実装に戻しました。
今見ると、そんなに共通化するような場所は実はなくなっていたのでした。
void EnemyA_Action()
{
Bark();
OpenUp(); // 「ガードを崩す」という知識を共通化
Kick();
Tumble();
Back();
Tumble();
}
void EnemyB_Action()
{
Bark();
Beam();
OpenUp(); // 「ガードを崩す」という知識を共通化
Kick();
Back();
Beam();
}
void EnemyC_Action()
{
Bark();
OpenUp(); // 「ガードを崩す」という知識を共通化
Kick();
Tumble();
Blast();
}
もし「ガードを崩すのに必要なパンチはやっぱり３発にしよう」と仕様変更があっても、OpenUp関数を直せばOKです。エンバグや修正漏れはありません。また、Enemy_CommonActionという関数名であったころは関数の中身を見に行かないと何をやっているのか全く不明でしたが、OpenUpならば中身を見なくてもある程度分かるというメリットも大きいです。
関数の命名でよくないDRYを弁別できるというのはあまり聞かない観点ですが、なかなか有用だと思っているのでオススメです。（そもそも関数などの命名に気を付けていればよくないDRYを引き起こしにくい、と捉えるならばいろんな文献に書いてある内容ではありますが。）