Code smell - which design pattern and how to implement?

Question

I think the following might be a good fit for the Strategy pattern but I'm not sure if that's correct and if so, how to properly implement.

I have a method that will accept a Category and a Sort. Currently Category and Sort are enums, so something like this:

public enum Category{
  Hot,
  New
}

public enum Sort{
  New,
  Rising,      
  ThisWeek
}

public String getUrl(Category category, Sort sort){
   validateCategorySortCombo(category, sort);
}

Notice the validateCategorySortCombo(Category, Sort). First I must validate the combination of Category + Sort as some are invalid. For example:

if(Category == "Hot" && Sort == "Rising"){
    ThrowInvalidCategorySortCombo("Can't use hot category with rising sort")
}  

There will be several of these checks to ensure the combinations are valid. This seems like a code smell because as other categories and sorts are introduced, I would have to open this class up and modify it which breaks the open/closed principle.

The second code smell comes when I have to inspect both Category and Sort to construct my Url which will be passed back. Currently I'm using a series of switch statements:

String url;
switch(category){
  case Hot:
     url += "www.site.com/hot/";
  case New:
     url += "www.site.com/new/";
}

switch(sort){
  case New:
    url += "?sort=rising";
  case ThisYear:
    url += "?sort=thisyear";
}

Again, as new categories and sorts are introduced, I would have to open this class and update the switch statements.

Couple of questions:

1. What design pattern would be used to solve my problem?
2. How would I go about implementing the design pattern?

Answer 1

I noticed that you did not only validate inputs but compose a URL string based on them. It might be a little overusing though looks like a classic case of Abstract Factory. You may have abstract families of Categories and concrete implementations SortedCategories. Other benefit would be that you can explicitly raise an exception in HotRaisedFactory as unacceptable combination of that category and that Sort.

Answer 2

For your 1st concern -

You can define a validator which can specifically be a hashmap whose key will be composition of Category and Sort.

you can upfront populate this map with valid combinations [hard coded/ config file driven/ periodic refresh etc can be thought upon].

In your validateCategorySortCombo you can check if the hashmap actually contains the key ? If yes, good to go.

--

Map<Compositekey<Sort,Category>,Boolean> validOptions = new hashMap..

void validateCategorySortCombo(category, sort){

  if(validOptions.containskey(category,sort)){
     //good to go
  }
}

Answer 3

First, I would suggest that you leave your mind open to multiple design patterns until you're writing some code. Allow your code to tell you which pattern would work best. If you pick one before hand, you could end up trying to 'force' your code to use a pattern that may not have been the best option.

Having that said, a couple of patterns that may end up being useful in your situation would be Strategy and Chain of Responsibility.

Moving on...

I think you need to take a step back and come up with what your design goals are based on how you expect the system to change over time. For example, it sounds like you're definitely expecting to be adding new Categories and Sorts. Based on that, two goals your design may have would be:

1. minimize modifications to validation code when adding a new Sort/Category
2. minimize modifications to url building code when adding a new Sort/Category

From those goals, come up with a generic approach. For example, my thoughts are...

1. If I separate each validation out into its own class, I won't need to modify existing validation algorithms when adding a new Category/Sort. All I'd have to do is add new validation algorithm or possibly delete an existing one if it no longer applies. (This could be overkill, but I don't know how complicated your validation algorithms are or will get)
2. If each Category/Sort knows how to provide its own piece of the url, each new category or sort should have no impact on the ability to get the url representations of existing categories or sorts.

Now you can start thinking more about implementation (where you have many options). For example, you could expand your enums so that each value already knows its url component like so:

public enum Sort{
  New("?sort=new"),
  Rising("?sort=rising"),      
  ThisWeek("?sort=thisweek");


  private String urlComponent;
  public Sort(String urlComponent) {this.urlComponent = urlComponent;)
  public getUrlComponent() {return this.urlComponent;}
}

Going this route, you could always call mySort.getUrlComponent(), which wouldn't need to change when adding/removing Sort enum values. This would address your second concern specifically.

I'm not going to be able to give you the best implementation or set of design patterns because I don't know everything you do about the system you're building, but I hope my thoughts here will help.

Answer 4

like M Platvoet said:

import java.util.*;
enum Category {
    Hot(EnumSet.of(Sort.Rising)),New(EnumSet.of(Sort.ThisWeek,Sort.ThisYear));
    Category(EnumSet<Sort> legal) {
        this.legal.addAll(legal);
    }
    boolean isLegal(Sort sort) {
        return legal.contains(sort);
    }
    private final EnumSet<Sort> legal=EnumSet.noneOf(Sort.class);
}
enum Sort {
    New,Rising,ThisWeek,ThisYear;
}
public class So9706550 {
        static boolean isValid(Category category,Sort sort) {
            return category.isLegal(sort);
    }
    public static void main(String[] args) {
        System.out.println(isValid(Category.Hot,Sort.Rising));
        System.out.println(isValid(Category.Hot,Sort.ThisWeek));
    }
}

Answer 5

Simply move the validation of the sort order to your Category enum. So that it reads category.isValidSort(sort);