リファクタリング中に識別子の名前をなおすべきか


 以下の記事は、テストが自動化されていない環境、バージョン管理にgitよりも古いツールを使っていたとき、リファクタリングしやすい統合環境を使っていなかった時代に書いたものです。
 ですから、リファクタリングが日常化している開発チームの状況では、この文章はまったく無用のものです。


 ソースコードのメンテナンスしていると、識別子の名前を直したいことは多々ある。スペリングが間違っている、用語の不統一、「まずい名前」などさまざまな理由がある。「テスト駆動開発による組み込みプログラミング」(オライリー)によれば、名前の変更がリファクタリングの要素にあげられている。
 しかし、今、私は本当に識別子を直すべきか迷っている。不完全なテストの状況において識別子を直さないほうが良いのではないかと。

直すべきでない理由1:違いは少ないほどよい

 ファイルを比較するとき変更点は、違いが少ないほどよい。違いの数が多いほど、反映させていい違いなのか、無視してよい違いなのかがわからなくなりやすい。たくさんの変更を行うと間違いをしでかす可能性が増える。ブレースと改行のコーディングスタイルなどを気にしなくてもよい違いをdiffで表示させてしまうことは避けたい。

直すべきでない理由2:変更漏れと変更のしすぎ

 変数名、データメンバー名を変更するときには、間違えて、一部だけ置き換えてしまったり、論理的には別の変数まで名前を変更してしまったりする。いつの間にか、論理構造を壊してしまってしまう。なんかおかしいなと思って、昔の版をSVNから取り出して、間違った改変をしていたことに気づく。クラスを用いていて、データメンバーは同一の名前を複数のクラスで使っていることが多い。だから、そのような間違いもしでかしやすい。
 スペリングミスであっても、つじつまがあっている限り、当面はそのままにしておくことがお勧めだと思っている。

直すべきでない理由3:名前がソースをたどるヒントでありつづける

 マクロ定数ではなく、enum型になった場合でも、識別子の名前は変えないままにしておくのが、当面望ましいと私は考える。グローバル変数から、クラスのデータメンバーに変わった場合でも、検索してたどりやすいというただ一点の理由のために、名前を変えないままにしておくのが、当面望ましいと私は考えている。
 以下の記事は、テストが自動化されていない環境、バージョン管理にgitよりも古いツールを使っていたとき、リファクタリングしやすい統合環境を使っていなかった時代に書いたものです。
 ですから、リファクタリングが日常化している開発チームの状況では、この文章はまったく無用のものです。

直すべきでない理由4:単体テストするまで待て

 プログラムは正しいままに保つことが大切であり、正しい状況にあることをテストして確認する。しかしながら、引継いだばかりのソースコードには、単体テストしやすい枠組みになっていないことが多い。テストしやすい界面を作り出すために、関数を抽出したりすることが多い。ただでさえそのような改変は、プログラムにバグを埋め込みやすい。
 引き継いで開発にかかわったソースは、意図を理解していない部分が多い。それもバグを埋め込みやすい原因の一つだ。テストが十分にできていない段階では、識別子の名前を変えるためだけの改変はすべきではないと考える。

直すべきでない理由5:単体テストしてあれば安心して名前を変えられる

 ロジックの妥当性を単体テストの枠組みに入れ込んでしまうまで、なるべく名前を変えないままにしておきたい。そして一群の改変が単体テストで妥当であることを検証しきるまで、そのままにしておきたいと思う。
 そして妥当性を単体テストで検証しきってからは、その識別子の名前を自信をもって改変できる。

直すべきでない理由6:追加のソースコードの出現

 メンテナンスすべきコードが、当初思っていた範囲よりも広範囲になものであったことが判明すると、この変数や関数の前の名前はなんだったのだろうということになる。

回避策:古い名称の関数は、廃止予定の関数として残しておいた方がよいのかもしれない。
 また、新しい枠組みになっても、一連の改変以前に使われていたインタフェースを持つ関数を、新しい枠組みに基づいて実装しておいた方がよいかもしれない。

 引数のデータ構造や戻り値が変更になって、関数の仕様が著しく変わったときは、従来の関数名に類似の関数名で別の仕様の関数として実装したほうがいいかもしれない。そうすれば、関数の仕様が変わった別物であることがわかるし、従来のインタフェースを廃止予定の仕様として残しておくことも可能になる。

注意:ここで述べているのは小規模の人数による開発です。開発プロジェクトの性格や方針によっては、ここに書いている内容はまったく役に立たないでしょう。

その後:改変済みコードの検索ツール(自作)

 関数を改変して古い関数が新しい版では別の関数名になっていて、しかも古い版の関数を使っているコードを読まなければならなくなったとき、次のようなスクリプトを用いて、対応する今の関数を見つけることにしている。
{古い関数名:新しい関数名, }
からなる辞書(=map)を用意しておいて、
ソースコード中に、古い関数名があるかどうかを検索するツールを自作した。
 移植済みの関数を再度移植してしまうことは防げるだろう。

直すべき理由

 識別子の名前から期待する内容と、その識別子の実際の内容とがかけ離れていれば、そのかけ離れていることが何度も「ああそうなんだっけ」という思いをするならば、ソースコードを共有していると相談して、識別子の内容を変更すべきだろう。何度も何度も、「本当はこういう意味なんだ」という思いを繰り返すことは、誤解に基づくコードを書きやすくしてしまう。また、Doxygenのcalltreeで表示させるのも関数名(識別子のひとつ)であり、誤解を発生させないためにも直すべきだと考える。

識別子の名前を変えるときには

 バージョン管理しているときには、識別子の名前の変更はそれだけでコミットしよう。識別子の変更とロジックの変更を同時にしてしまうと、何か問題を生じたときにどの変更が問題を生じたのかをたどりにくくする。だから、識別子の名前の変更はそれだけでコミットしている。
 変更の影響が少ないのは、ローカル変数の名前、関数定義の引数の名前の変更、関数名の変更の順であるから、変更の影響が大きい改変には少しばかり慎重になる。リファクタリングが落ちついてきて、関数の動作にも自信が持てるようになってきたら、普通にリファクタリングの本が勧めているようにわかりやすい、誤解をまねきにくい名前に変更しよう。