2

前回は、いくつかのパラメーターを持ち、そのうちの 1 つだけを使用する長い関数をよく書きました。機能は、関数の周りに散らばっているいくつかのキーポイントでのみ異なります。したがって、関数を分割すると、目的のない小さな関数が多すぎます。これは良いスタイルですか、それともこれに適した一般的なリファクタリング パターンはありますか? より明確にするために、例：

public performSearch(DataBase dataBase, List<List<String>> segments) {performSearch(dataBase,null,null,segments);}
public performSearch(DataBaseCache dataBaseCache,List<List<String>> segments) {performSearch(null,dataBaseCache,null,segments);}
public performSearch(DataBase dataBase, List<String> keywords {performSearch(dataBase,null,keywords,null);}
public performSearch(DataBaseCache dataBaseCache,List<String> keywords) {performSearch(null,dataBaseCache,keywords,null);}

/** either dataBase or dataBaseCache may be null, dataBaseCache is used if it is non-null, else dataBase is used (slower). */
private void performSearch(DataBase dataBase, DataBaseCache dataBaseCache, List<String> keywords, List<List<String>> segments)
{
 SearchObject search = new SearchObject();
 search.setFast(true);
 ...
 search.setNumberOfResults(25);

 if(dataBaseCache!=null) {search.setSource(dataBaseCache);}
 else                    {search.setSource(dataBase);}

 ... do some stuff ...
 if(segments==null) 
 {
  // create segments from keywords 
  ....
  segments = ...
  }
}

このスタイルのコードは機能しますが、これらすべての null パラメーターと、このようなメソッドを間違って呼び出す可能性は好きではありません (両方のパラメーターが null、両方が null でない場合に何が起こるか)。 ...これは一般的すぎるかもしれませんが、誰かがこの問題の原則に対する一般的な解決策を持っているかもしれません:-)

PS: 長い関数以外に理由がない場合 (つまり、サブ関数がその順序でのみ、この 1 つの関数によってのみ呼び出される場合)、特にそれらが密に絡み合っている場合、長い関数を分割するのは好きではありません。それらの周りに大量のパラメーターを転送する必要があります。

4

4 に答える 4

2

非常に悪い手続き型だと思います。そのようなコーディングは避けるようにしてください。すでにそのようなコードが大量にあるため、各メソッドには他とはわずかに異なる独自のロジックが含まれているため、リファクタリングが非常に難しい場合があります。ところで、それが難しいという事実は、スタイルが悪いという証拠です.

次のような行動パターンを使用する必要があると思います

  1. 責任の連鎖
  2. 指示
  3. ストラテジー
  4. テンプレート方式

これは、手続き型コードをオブジェクト指向に変更するのに役立ちます。

于 2012-09-19T13:04:41.630 に答える
1

このようなものを使用できますか

public static <T> T firstNonNull(T...parameters) {
    for (T parameter: parameters) {
        if (parameter != null) {
            return parameter;
        }
    }
    throw new IllegalArgumentException("At least one argument must be non null");
}

複数のパラメーターがnullでなく、同じタイプである必要があるかどうかはチェックされませんが、次のように使用できます。

search.setSource(firstNonNull(dataBaseCache, database));
于 2012-09-19T13:01:53.083 に答える
1

null を期待することはアンチパターンです。これは、NullPointerExceptions が発生するのを待っているコードが散らばっているためです。ビルダー パターンを使用してSearchObject. これはあなたが望む署名です。私はあなたに実装を理解させます:

class SearchBuilder {
   SearchObject search = new SearchObject();
   List<String> keywords = new ArrayList<String>();
   List<List<String>> segments = new ArrayList<List<String>>();

   public SearchBuilder(DataBase dataBase) {}
   public SearchBuilder(DataBaseCache dataBaseCache) {}
   public void addKeyword(String keyword) {}
   public void addSegment(String... segment) {}

   public void performSearch();
}
于 2012-09-19T13:19:33.083 に答える
0

私はアレックスが言ったことに同意します。問題を知らずに、例の内容に基づいて次の構造をお勧めします。

public interface SearchEngine {
  public SearchEngineResult findByKeywords(List<String> keywords);
}

public class JDBCSearchEngine {
  private DataSource dataSource;

  public JDBCSearchEngine(DataSource dataSource) {
     this.dataSource = dataSource;
  }

  public SearchEngineResult findByKeywords(List<String> keywords) {
     // Find from JDBC datasource
     // It might be useful to use a DAO instead of datasource, if you have database operations other that searching
  }
}

public class CachingSearchEngine {
  private SearchEngine searchEngine;

  public CachingSearchEngine(SearchEngine searchEngine) {
    this.searchEngine = searchEngine;
  }

  public SearchEngineResult findByKeywords(List<String> keywords) {
    // First check from cache
    ...
    // If not found, then fetch from real search engine
    SearchEngineResult result = searchEngine.findByKeywords(keywords);
    // Then add to cache
    // Return the result
    return result;
  }
}
于 2012-09-19T13:21:16.667 に答える