CODEWITHSUNDEEP
javaparametersnull
Konrad Höffner
Konrad Höffner

Reputation: 12207

Java pattern for parameters of which only one needs to be non-null?

In the last time I often write long functions that have several parameters but use only one of them and the functionality is only different at a few keypoints that are scattered around the function. Thus splitting the function would create too many small functions without a purpose. Is this good style or is there a good general refactoring pattern for this? To be more clear, an example:

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 = ...
  }
}

This style of code works but I don't like all those null parameters and the possibilities of calling methods like this wrong (both parameters null, what happens if both are non-null) but I don't want to write 4 seperate functions either... I know this may be too general but maybe someone has a general solution to this principle of problems :-)

P.S.: I don't like to split up a long function if there is no reason for it other than it being long (i.e. if the subfunctions are only ever called in that order and only by this one function) especially if they are tightly interwoven and would need a big amount of parameters transported around them.

Upvotes: 2

Views: 415

Answers (4)

Garrett Hall
Garrett Hall

Reputation: 30022

Expecting nulls is an anti-pattern because it litters your code with NullPointerExceptions waiting to happen. Use the builder pattern to construct the SearchObject. This is the signature you want, I'll let you figure out the implementation:

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();
}

Upvotes: 1

Sami Korhonen
Sami Korhonen

Reputation: 1303

I agree with what Alex said. Without knowing the problem I would recommend following structure based on what was in the example:

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;
  }
}

Upvotes: 0

AlexR
AlexR

Reputation: 115328

I think it is very bad procedural style. Try to avoid such coding. Since you already have a bulk of such code it may be very hard to re-factor it because each method contains its own logic that is slightly different from other. BTW the fact that it is hard is an evidence that the style is bad.

I think you should use behavioral patterns like

  1. Chain of responsibilities
  2. Command
  3. Strategy
  4. Template method

that can help you to change your procedural code to object oriented.

Upvotes: 2

Roger Lindsj&#246;
Roger Lindsj&#246;

Reputation: 11543

Could you use something like this

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");
}

It does not check if more than one parameter is not null and they must be of the same type, but you could use it like this:

search.setSource(firstNonNull(dataBaseCache, database));

Upvotes: 1

Related Questions