リファクタリングは大事です

Test::Module::Usedの version 0.0.6 をリリースしました。機能的にはほとんど変わっていません。黙々とコードをリファクタリングしてただけ。でも、その過程でバグを見つけちゃいました。。。

以前は条件式で、コアモジュールかどうかの判定をしていたのを、「メソッドの抽出」で取り出して、(メソッド名と意味が反対になるので)条件式を反転して、ド・モルガンの法則適用したらあら不思議。

以前の条件(コアモジュールに含まれない)

my $first_release = Module::CoreList->first_release($module);
push @result, $module if ( !defined $first_release || $first_release >= $version );

抽出後の条件(コアモジュールに含む、間違い版)

my $first_release = Module::CoreList->first_release($module_name);
return defined $first_release && $first_release < $self->_version

ん？「$first_release < $self->_version」だと、コアに含まれたちょうどのリリースが除外されちゃうぞ？？、というわけでバグ発見でした。

正解はこうですね。

抽出後の条件(コアモジュールに含む、正解)

my $first_release = Module::CoreList->first_release($module_name);
return defined $first_release && $first_release <= $self->_version

実際のコードではこんな感じになっています。

% git diff version-0.0.5 version-0.0.6 lib/Test/Module/Used.pm

(より抜粋)

sub _remove_core {
-    my( $version, @modules ) = @_;
-    my @result;
-    for my $module ( @modules ) {
-        my $first_release = Module::CoreList->first_release($module);
-        push @result, $module if ( !defined $first_release || $first_release >= $version );
-    }
+    my $self = shift;
+    my( @module_names ) = @_;
+    my @result = grep {  !$self->_is_core_module($_) } @module_names;
     return @result;
 }

+sub _is_core_module {
+    my $self = shift;
+    my($module_name) = @_;
+    my $first_release = Module::CoreList->first_release($module_name);
+    return defined $first_release && $first_release <= $self->_version;
+}
+

リファクタリングは大事ですね。