既にコメントが付いてるので、設計レベルでどうだとか、というのはおいておきます。
とにかく実装をパッと見て、これはダメそうだなというのは
52 sub lock {
中略
62 $self->unlock;
63 }
64 }
65 if (symlink $target, $self->file) {
あたりで気づけるのではないでしょうか。lock関数の中で、実際にlockを獲得しているのが65行目のsymlinkなのに、その手前でunlockを呼んでます。一般的に言って、獲得してないロックを解放するのはかなりまずいでしょう(実際、このunlockの呼び出しの直前で他のプロセスがlockに成功すると、そこで生成されたロックファイルを勝手に削除しちゃうことになり、結局2重にlockが獲得できちゃいます)。
さて、標題の「関数名は大事だよね」ですが、unlockの実装は
73 sub unlock {
74 unlink $_[0]->file;
75 }
だけなので、下手するとunlinkってな名前を付けちゃいそうです。が、実装がたまたまそうなってるだけで、呼び出し側から見ればやりたいことはunlockなのですから、この名前で大正解。これがunlockと命名されていたからこそ、lock関数の実装がダメそうなのがすぐに分かったわけです。
というわけで、関数名を横着しないできちんと付けてると、コードの間違にも気づきやすいよねというお話でした。
0 件のコメント:
コメントを投稿