mixiグループ Advent Calendar 2016用の記事です
そこそこ大きな Web ページ等を運営していると、
- ModelA でバグの温床になりそうなコード発見(でも今のところ正しく動いている??)
- ModelA を使っているページが20ページもある!!!
- リファクタするのも正しく動くか確認するのも大変だ
こんなことがよくあります。
そんな場合のプラクティスを紹介いたします。
ケース
# ModelA
sub is_kuso_code {
my $self = shift;
my $is_kuso = ...;
return $is_kuso ? 'y' : 'n'; # bool ぽい値じゃなくて文字列の y/n を返している・・だと!
}
上記のようなクソコードが有ったとします。
ちなみにこいつ if (is_kuso_code()) ..
とすると全てをクソコードとして判定してくれます
DB に enum で y/n とか格納しているのを引っ張ってくると稀にこういうことが起こるかもしれません。
対応
まず前提としてこれらは各モジュールで UnitTest が書かれているべきです。それは当たり前にやっているとして、どうやってこいつをリファクタしていくかを考えます。
全てをいっぺんに直すのは実装もチェックも大変なので少しずつ直しましょう。
全てを直す!!!と息ごんでも途中で挫折してこの負債が残り続けるのがオチです。
1. これ以上負債への依存が増えないようする
既存の挙動を変えないようにしつつ、新規に使おうとすると自然と良い方の仕様が使われるようにします。
ここでは uses_deprecated_kuso_condition_output
という長ったらしくていかにも怪しげなプロパティを ON にすると、 y/n という文字列を返すようにしましょう。
(もしくは、 is_kuso_code()
を is_legacy_kuso_code()
など改名して新規に is_kuso_code()
を作るのでも構いません。)
# ModelA
# モデルのプロパティ
__PACKAGE__->mk_accessors(qw/
uses_deprecated_kuso_condition_output
/);
sub is_kuso_code {
my $self = shift;
my $is_kuso = ...;
if ($self->uses_deprecated_kuso_condition_output) {
# こっちは新規では使わないでね
return $is_kuso ? 'y' : 'n';
} else {
# perl では true/false は 1/0 とか 1/undef とかだよ
return $is_kuso ? 1 : 0;
}
}
ModelA を改修したら、そいつを使っている部分を全て uses_deprecated_kuso_condition_output: true にしていきます。
# some page
- my $modelA = ModelA->new();
+ my $modelA = ModelA->new({uses_deprecated_kuso_condition_output => 1});
my $is_kuso = $modelA->is_kuso_code();
if ($is_kuso == 'y') {
...
これで ModelA を使っているページの挙動を変えることなく、新規に ModelA が使われるときには悪い使われ方をしないようになりました。
とりあえず、これで状況はこれ以上悪くはならなくなりました。
(テストが書かれていないと、これで本当に影響が無いのか・・とかが自信がなくなりますので、テストはある前提です。)
2. 既存の古い使われ方をしている場所を直していく
uses_deprecated_kuso_condition_output とかで grep して、時間をかけて少しずつこのプロパティを使っている部分を直していきます。
一つづつやっていくことによって、動作確認の範囲も小さくできますので事故の少ないリファクタができます。
また、小分けにすることによって空いた小さな時間でリファクタを進めていくことや、複数人で並列して負債の返済をしていくことができます。