refactoring - コンパイルエラーによるリファクタリングは悪いことですか?

Question

私は、コンパイルエラーを導入することによって、いくつかのリファクタリングを行うことに慣れてきました。たとえば、クラスからフィールドを削除し、それをいくつかのメソッドのパラメーターにしたい場合、通常は最初にフィールドを削除しますが、これによりクラスのコンパイルエラーが発生します。次に、パラメーターをメソッドに導入すると、呼び出し元が壊れます。等々。これは通常、私に安心感を与えました。私は実際にリファクタリングに関する本を (まだ) 読んだことがありませんが、以前はこれが比較的安全な方法だと思っていました。でも、本当に安全なのだろうか？それともやり方が悪いのでしょうか？

score 10 · Accepted Answer

リファクタリング時に単純なコンパイルに頼ることはありません。コードはコンパイルできますが、バグが導入されている可能性があります。

リファクタリングしたいメソッドやクラスの単体テストだけを書くのが一番良いと思います。リファクタリング後にテストを実行することで、バグが導入されていないことを確認できます。

テスト駆動開発に行くと言っているのではありません。単体テストを書いて、リファクタリングに必要な信頼を得ることができます。

score 9 · Accepted Answer

これは、静的にコンパイルされた言語の一般的で便利な手法です。あなたがしていることの一般的なバージョンは、次のように述べることができます：

そのモジュールのクライアントでの一部の使用を無効にする可能性があるモジュールに変更を加える場合は、コンパイル時エラーが発生するような方法で最初の変更を行います。

さまざまな結果があります。

メソッド、関数、またはプロシージャの意味が変わり、型も変わらない場合は、名前を変更します。(すべての用途を精査して修正したら、おそらく名前を元に戻すでしょう。)
データ型に新しいケースを追加するか、列挙に新しいリテラルを追加する場合は、既存のすべてのデータ型コンストラクタまたは列挙リテラルの名前を変更します。(または、ケース分析が網羅的かどうかを確認するコンパイラを持っている幸運な場合は、もっと簡単な方法があります。)
オーバーロードのある言語で作業している場合は、1 つのバリアントを変更したり、新しいバリアントを追加したりしないでください。オーバーロードが別の方法でサイレントに解決されるリスクがあります。オーバーロードを使用すると、コンパイラを希望どおりに動作させるのは非常に困難です。オーバーロードを処理するために私が知っている唯一の方法は、すべての使用についてグローバルに推論することです。IDE が役に立たない場合は、オーバーロードされたすべてのバリアントの名前を変更する必要があります。不快。

実際に行っているのは、コンパイラを使用して、変更が必要な可能性のあるコード内のすべての場所を調べることです。

score 5 · Accepted Answer

問題はないと思います。安全であり、コンパイルする前に変更をコミットしない限り、長期的な影響はありません。また、Resharper と VS には、プロセスを少し簡単にするツールがあります。

TDD では、同様のプロセスを別の方向に使用します。メソッドが定義されていない可能性があるコードを作成すると、コンパイルされず、コンパイルするのに十分なコードを作成します (その後、テストに合格するなど...)。

score 5 · Accepted Answer

このテーマに関する本を読む準備ができたら、Michael Feather の「レガシーコードを効果的に使用する」をお勧めします。(非著者による追加: Fowler の古典的な本 " Refactoring " と、Refactoring Web サイトも役立つかもしれません。 )

彼は、変更を加える前に作業中のコードの特性を特定し、スクラッチリファクタリングと彼が呼ぶことを行うことについて語っています。それは、コードの特徴を見つけるためにリフェクトし、結果を捨てることです。

あなたがしていることは、コンパイラを自動テストとして使用することです。コードがコンパイルされることをテストしますが、リファクタリングのために動作が変更されたかどうか、または何らかの副作用があったかどうかはテストしません。

このことを考慮

class myClass {
     void megaMethod() 
     {
         int x,y,z;
         //lots of lines of code
         z = mysideEffect(x)+y;
         //lots more lines of code 
         a = b + c;
     }
}

追加をリファクタリングできます

class myClass {
     void megaMethod() 
     {
         int a,b,c,x,y,z;
         //lots of lines of code
         z = addition(x,y);
         //lots more lines of code
         a = addition(b,c);  
     }

     int addition(int a, b)
     {
          return mysideaffect(a)+b;
     }
}

これは機能しますが、メソッドを呼び出すため、2番目のアドオンは間違っています。コンパイルだけでなく、さらにテストが必要になります。

score 4 · Accepted Answer

コンパイラエラーによるリファクタリングが黙って失敗し、意図しない結果が生じる例について考えるのは簡単です。
頭に浮かぶいくつかのケース: (C++ について話していると仮定します)

既定のパラメーターを使用して他のオーバーロードが存在する関数への引数の変更。リファクタリング後、引数の最適な一致が期待どおりにならない場合があります。
キャスト演算子または非明示的な単一引数コンストラクターを持つクラス。これらのいずれかの引数を変更、追加、削除、または変更すると、関連するコンスタレーションに応じて、呼び出される最適な一致が変更される可能性があります。
仮想関数を変更し、基本クラスを変更しない (または基本クラスを別の方法で変更しない) と、呼び出しが基本クラスに向けられます。

コンパイラエラーに依存する方法は、必要なすべての変更をコンパイラが確実にキャッチできると確信している場合にのみ使用してください。私はほとんどの場合、これについて疑念を抱く傾向があります。

score 3 · Accepted Answer

ここにあるすべての知恵に加えて、これが安全ではない可能性がある別のケースがあることを付け加えたいと思います. 反射。これは、.NET や Java などの環境に影響します (もちろん、他の環境にも影響します)。コードはコンパイルされますが、Reflection が存在しない変数にアクセスしようとすると、実行時エラーが発生します。たとえば、Hibernate のような ORM を使用していて、マッピング XML ファイルを更新するのを忘れている場合、これはよくあることです。

コードファイル全体で特定の変数/メソッド名を検索する方が少し安全かもしれません。もちろん、多くの誤検知が発生する可能性があるため、普遍的な解決策ではありません。また、リフレクションでも文字列連結を使用する可能性があり、これも役に立たなくなります。しかし、少なくとも安全に一歩近づくことはできます。

ただし、すべてのコードを手動で実行する以外に、100% 誰にでもできる方法があるとは思いません。

score 2 · Accepted Answer

dotnet lanuages のいずれかを使用している場合に検討できるもう 1 つのオプションは、「古い」メソッドに Obsolete 属性を付けてフラグを立てることです。これにより、すべてのコンパイラ警告が導入されますが、コードが呼び出し可能のままになります。 control (たとえば、API を作成している場合、または VB.Net でオプション strict を使用しない場合)。廃止されたバージョンが新しいバージョンを呼び出すように、楽しくリファクタリングすることができます。例として：

    public string Username
    {
        get
        {
            return this.userField;
        }
        set
        {
            this.userField = value;
        }
    }

    public int Login()
    {
        /* do stuff */
    }

なる:

    [ObsoleteAttribute()]
    public string Username
    {
        get
        {
            return this.userField;
        }
        set
        {
            this.userField = value;
        }
    }

    [ObsoleteAttribute("Replaced by Login(username, password)")]
    public int Login()
    {
        Login(Username, Pasword);
    }

    public int Login(string username, string password)
    {
        /* do stuff */
    }

とにかく、私はそれをする傾向があります...

score 2 · Accepted Answer

その特定のものへのすべての参照を見つけるので、それは私が考えるかなり一般的な方法です。ただし、Visual Studio などの最新の IDE には、これを不要にするすべての参照の検索機能があります。

ただし、このアプローチにはいくつかの欠点があります。大規模なプロジェクトの場合、アプリケーションのコンパイルに時間がかかる場合があります。また、これを長時間実行しないでください (つまり、できるだけ早く機能を元に戻すことを意味します)。また、一度に複数のことを実行しないでください。パッチを適用した正しい方法を忘れる可能性があります。初めて。

score 2 · Accepted Answer

これは 1 つの方法であり、リファクタリングしているコードがどのように見え、どのような選択をしたかを知らなければ、それが安全か安全でないかについて明確な声明を出すことはできません。

うまくいっているのであれば、変更のためだけに変更する理由はありませんが、時間があれば、ここにあるリソースを読んで、時間の経過とともに探求したい新しいアイデアが得られるかもしれません.

http://www.refactoring.com/

score 1 · Accepted Answer

コンパイル時に十分にチェックされた言語では、変更したものへのすべてのライブ参照を更新する必要があるという意味で「安全」です。

条件付きでコンパイルされたコードがある場合、たとえばC / C ++プリプロセッサを使用している場合は、それでも問題が発生する可能性があります。したがって、可能なすべての構成と、該当する場合はすべてのプラットフォームで再構築するようにしてください。

変更をテストする必要がなくなるわけではありません。関数にパラメーターを追加した場合、コンパイラーは、その関数の各呼び出しサイトを更新したときに指定した値が正しい値であったかどうかを判断できません。パラメータを削除した場合でも、次のように変更するなど、間違いを犯す可能性があります。

void foo(int a, int b);

に

void foo(int a);

次に、呼び出しを次の場所から変更します。

foo(1,2);

に：

foo(2);

それはうまくコンパイルされますが、それは間違っています。

個人的には、変更する関数へのライブ参照をコードで検索する方法として、コンパイル（およびリンク）の失敗を使用します。ただし、これは単なる省力化デバイスであることに注意する必要があります。結果のコードが正しいことを保証するものではありません。

score 1 · Accepted Answer

これは、テスト駆動開発で使用される完全に標準的な方法に似ています。つまり、存在しないクラスを参照するテストを記述して、テストに合格するための最初のステップがクラスを追加し、次にメソッドを追加する、というようにします。完全な Java の例については、 Beck の本を参照してください。

あなたのリファクタリングの方法は危険に思えます。なぜなら、安全性に関するテストがないからです (または、少なくともテストがあることを言及していないからです)。実際には意図したとおりに動作しないコンパイルコードを作成したり、アプリケーションの他の部分を壊したりする可能性があります。

実践に簡単なルールを追加することをお勧めします: make non-compiling changes only in unit test code . そうすれば、変更ごとに少なくともローカルテストを確実に行うことができ、変更を行う前に変更の意図をテストに記録できます。

ところで、Eclipse は、この「失敗、スタブ、書き込み」メソッドを Java でとてつもなく簡単にします。存在しないオブジェクトはそれぞれマークされ、Ctrl-1 とメニュー選択は、(コンパイル可能な) スタブを作成するように Eclipse に指示します! 他の言語や IDE が同様のサポートを提供しているかどうか知りたいです。

score 1 · Accepted Answer

これは一般的なアプローチですが、言語が静的か動的かによって結果が異なる場合があります。

静的に型付けされた言語では、導入した不一致はコンパイル時に検出されるため、このアプローチはある程度理にかなっています。ただし、動的言語は多くの場合、実行時にのみこれらの問題に遭遇します。これらの問題は、コンパイラではなく、テストスイートによって検出されます。あなたが書いたと仮定します。

C# や Java などの静的言語を使用しているように見えるので、何らかの重大な問題が発生するまで、このアプローチを続けてください。

score 1 · Accepted Answer

私は通常のリファクタリングを行いますが、コンパイラエラーを導入してリファクタリングを行います。私は通常、変更がそれほど単純ではない場合、およびこのリファクタリングが実際のリファクタリングではない場合に行います (機能を変更しています)。これらのコンパイラエラーは、名前やパラメーターの変更よりも複雑な変更を行う必要がある箇所を示しています。

refactoring - コンパイルエラーによるリファクタリングは悪いことですか?

13 に答える 13

Related

Reference