2

単一のメソッドでクエリを実行しているデータがいくつかあります。矢じりのアンチパターンになるところまで来ています。次のようになります。

void queryData()
{
    int masterIndex = getMasterIndex();
    if (masterIndex != -1)
    {
        byte[] pageData = getMasterPage(masterIndex);
        if (pageData) != null)
        {
            Item1 i1 = getItem1(pageData);
            Item2 i2 = getItem2(pageData);

            if (i1 != null && i2 != null)
            {
                showResults(i1, i2);
            }
        }
    }
}

上記を想像してみてください。より多くの if ステートメントと、呼び出される各メソッドには、適切な量のロジックが含まれています。

今私ができることは、上記のメソッドをリファクタリングして、すべての if ステートメントが肯定的で、true の場合は早期に返すことです。

ただし、各クエリと有効性チェックを独自のクラスに分割する方がクリーンだと思います。各アクションは、次のようなインターフェースを継承/実装します。

public interface Action
{
    public void run();
    public boolean wasSuccessful();
}

必要なアクションのリストを作成し、一度に 1 つずつ実行します。このようにして、各アクションに属するロジックが明確になります。

これは過剰に設計されていますか？上記は私がまだ知らない既存のパターンですか？

前もって感謝します。

4

2 に答える 2

4

IDE の「メソッドの抽出」機能 (ある場合) を悪用することから始め、各ロジック ブランチを独自のメソッドに引き出します。そうすれば、コードをより読みやすくすることができます。

リファクタリングの結果がコード自体のビジネス ロジックを壊したり変更したりしないことを確認するために、最初に単体テストの作成を開始することをお勧めします。より小さなメソッドにリファクタリングし、コードが当初の意図どおりに機能することを確認したら、クラスを作成してコードをそれらに抽出できるかどうかを確認できます。

理にかなっていて読みやすい限り、クエリと有効性チェックを行うクラスを作成することは過度に設計されているとは言えません。あなたが言ったように、それぞれでメソッドList<Action>を呼び出してループし、run()それぞれをチェックwasSuccessful()して、必要に応じて情報を出力することができます。

このように、特定のアクションの検証またはクエリを変更したい場合は、機能がカプセル化されているクラスを変更するだけで、実際の実行コードを変更する必要はありません。

于 2012-04-30T06:23:53.867 に答える
2

初期のリターンだけでどれだけきれいになったか見てみましょう:

void queryData()
{
    int masterIndex = getMasterIndex();
    if (masterIndex == -1) 
        return;
    byte[] pageData = getMasterPage(masterIndex);
    if (pageData == null)
        return;
    Item1 i1 = getItem1(pageData);
    Item2 i2 = getItem2(pageData);
    if (i1 == null || i2 == null)
        return;
    showResults(i1, i2);
}

これは、追加のクラス構造を作成するよりも優れたアプローチだと思います。

于 2012-04-30T10:57:27.263 に答える