書こうと思ったきっかけとか
昔、いわゆる長年掛けて熟成されたソースに手を入れる機会があり、ほぼswitch文だけで1000行みたいなCの関数を既存コードを前に奮闘してた記憶を掘り起こしながら、私が当時どのようにリファクタリングを行っていたのか、備忘録というか、メモ書きしておこうと思い立ったので。
冗長なswitch文
とりあえず、以下のような敵種別と敵ステータス型が定義されてるとして
typedef enum {
ENEMY_SLIME,
ENEMY_BIRD,
ENEMY_MAGE,
ENEMY_ORC,
ENEMY_DRAGON,
// なんだか他にもいっぱい
}EnemyType;
typedef struct {
char name[20];
int hp, mp, atk, def, ex, gold;
}EnemyStatus;
種別ごとに固有のステータスを取得する関数があったとします。
EnemyStatus GetEnemyStatus(EnemyType type) {
EnemyStatus status;
strcpy(status.name, "unknown");
status.hp = 1;
status.mp = 0;
status.atk = 0;
status.def = 0;
status.ex = 0;
status.gold = 0;
switch(type){
case ENEMY_SLIME:
strcpy(status.name, "slime");
status.hp = 30;
status.atk = 10;
status.ex = 1;
status.gold = 1;
break;
case ENEMY_BIRD:
strcpy(status.name, "bird");
status.hp = 80;
status.atk = 20;
status.def = 10;
status.ex = 5;
status.gold = 10;
break;
case ENEMY_MAGE:
strcpy(status.name, "mage");
status.hp = 50;
status.mp = 800;
status.atk = 10;
status.def = 30;
status.ex = 20;
status.gold = 20;
break;
case ENEMY_ORC:
strcpy(status.name, "orc");
status.hp = 300;
status.atk = 100;
status.def = 20;
status.ex = 30;
status.gold = 50;
break;
case ENEMY_DRAGON:
strcpy(status.name, "dragon");
status.hp = 800;
status.mp = 100;
status.atk = 150;
status.def = 80;
status.ex = 100;
status.gold = 300;
break;
// 以下延々と敵の種類数だけ続く
}
return status;
}
…うわぁ。もうね。なんかね。この時点で外部から読み込みなよ…と突っ込みたくもなるのですが、なにぶん熟成されたソースなのでひとまずはご了承ください。
文字位置を揃える
とりあえず、私がまずやるのがこれです。
何百何千行ものコードを整理する際、やはり一番こわいのは必要な処理まで消すことなので、まず処理に関わる文字には一切タッチせず、
スペースと改行だけを挿入・削除だけを行う
と決心します。そして
・case~breakの処理を1行にする(これいろんな人に怒られそう)
・言語のトークンレベルで各case内処理の左右位置を可能な限り揃える
という整理をします。こんな感じになります。
switch(type){
case ENEMY_SLIME : strcpy(status.name, "slime" ); status.hp = 30; status.atk = 10; status.ex = 1; status.gold = 1; break;
case ENEMY_BIRD : strcpy(status.name, "bird" ); status.hp = 80; status.atk = 20; status.def = 10; status.ex = 5; status.gold = 10; break;
case ENEMY_MAGE : strcpy(status.name, "mage" ); status.hp = 50; status.mp = 800; status.atk = 10; status.def = 30; status.ex = 20; status.gold = 20; break;
case ENEMY_ORC : strcpy(status.name, "orc" ); status.hp = 300; status.atk = 100; status.def = 20; status.ex = 30; status.gold = 50; break;
case ENEMY_DRAGON : strcpy(status.name, "dragon"); status.hp = 800; status.mp = 100; status.atk = 150; status.def = 80; status.ex = 100; status.gold = 300; break;
// 以下延々と敵の種類数だけ続く
}
…わぁ、なんか、すごく怒られそう。そして横に長い。
これはあれですね。
・1行に(セミコロンで区切られる)複数の文を書いてはならない。
・1行は半角80文字以内としなければならない。
みたいなコーディング規約があったら一発アウトですね。
デフォルト値を補完するなど
ENEMY_SLIMEのmpなどは、switchの外で初期化された0に設定されていますね。これをswitch内に補完してみましょう。
switch(type){
case ENEMY_SLIME : strcpy(status.name, "slime" ); status.hp = 30; status.mp = 0; status.atk = 10; status.def = 0; status.ex = 1; status.gold = 1; break;
case ENEMY_BIRD : strcpy(status.name, "bird" ); status.hp = 80; status.mp = 0; status.atk = 20; status.def = 10; status.ex = 5; status.gold = 10; break;
case ENEMY_MAGE : strcpy(status.name, "mage" ); status.hp = 50; status.mp = 800; status.atk = 10; status.def = 30; status.ex = 20; status.gold = 20; break;
case ENEMY_ORC : strcpy(status.name, "orc" ); status.hp = 300; status.mp = 0; status.atk = 100; status.def = 20; status.ex = 30; status.gold = 50; break;
case ENEMY_DRAGON : strcpy(status.name, "dragon"); status.hp = 800; status.mp = 100; status.atk = 150; status.def = 80; status.ex = 100; status.gold = 300; break;
// 以下延々と敵の種類数だけ続く
}
こうなりました。
データを抽出して処理と分離
ここまで揃えると、もはやそのままデータ配列として使えそうなテキストデータになりつつあるので、そのまま利用してデータ配列に切り出しちゃいます。
static const EnemyStatus EnemyStatusMaster[] = {
{ "slime" , 30, 0, 10, 0, 1, 1 },
{ "bird" , 80, 0, 20, 10, 5, 10 },
{ "mage" , 50, 800, 10, 30, 20, 20 },
{ "orc" , 300, 0, 100, 20, 30, 50 },
{ "dragon" , 800, 100, 150, 80, 100, 300 },
// 以下延々と敵の種類数だけ続く
{ "unknown", 1, 0, 0, 0, 0, 0 },
};
こんな風に。私のPCではサクラエディタの矩形選択機能(Shift+F6)がこの加工作業に大活躍してくれます。
すると処理は、もはやいろいろ要らなくなってしまいます。
EnemyStatus GetEnemyStatus(EnemyType type) {
static const EnemyStatus EnemyStatusMaster[] = {
{ "slime" , 30, 0, 10, 0, 1, 1 },
{ "bird" , 80, 0, 20, 10, 5, 10 },
{ "mage" , 50, 800, 10, 30, 20, 20 },
{ "orc" , 300, 0, 100, 20, 30, 50 },
{ "dragon" , 800, 100, 150, 80, 100, 300 },
// 以下延々と敵の種類数だけ続く
{ "unknown", 1, 0, 0, 0, 0, 0 },
};
return EnemyStatusMaster[type];
}
シンプル。
もはや構造体配列を参照させれば良い感じになってきました。
以下補足的な
** 基本は等価交換 **
私がこういった何百何千行ものコードを整理する場合
・ヒューマンエラーがこわいので、可能なだけ機械的に変換する
・数学で解を導くのと同様に、イコールで結べる等価なソースコードを経て段階的に直す
・何が固有なデータで、何が共通な処理かを見極める
といった点を留意してます。
** データをどこに置くか問題 **
切り出したデータを実際どう取り扱うかは、組込とかPCとか、環境に依存することになるかと思います。
・ファイル読込の処理時間が許されるなら、csvファイルに分割できるかもしれません
・ハードコーディングでいくなら、静的領域に置くようにするかもしれません
switch文でハードコーディングされている状態ではデータの外部化が難しいので、まずはプログラムからデータを抽出して切り離そう、という話でした。