Question

I'm currently working on a webapp where we often need to condition some server logic based on the page that is going to be returned to the user.

Each page is given a 4-letter page code, and these page codes are currently listed in a class as static Strings:

public class PageCodes {
    public static final String FOFP = "FOFP";
    public static final String FOMS = "FOMS";
    public static final String BKGD = "BKGD";
    public static final String ITCO = "ITCO";
    public static final String PURF = "PURF";
    // etc..
}

And often in the code we see code like this (1st form):

if (PageCode.PURF.equals(destinationPageCode) || PageCodes.ITCO.equals(destinationPageCode)) {
    // some code with no obvious intent
} 
if (PageCode.FOFP.equals(destinationPageCode) || PageCodes.FOMS.equals(destinationPageCode)) {
    // some other code with no obvious intent either
} 

Which is kind of horrible to read because it doesn't show what common property of these pages led the author of the code to put them together here. We have to read the code in the if branch to understand.

Current solution

These ifs have partly been simplified using lists of pages which are declared by different people in different classes. This makes the code look like (2nd form):

private static final List<String> pagesWithShoppingCart = Collections.unmodifiableList(Arrays.asList(PageCodes.ITCO, PageCodes.PURF));
private static final List<String> flightAvailabilityPages = Collections.unmodifiableList(Arrays.asList(PageCodes.FOMS, PageCodes.FOFP));

// later in the same class
if (pagesWithShoppingCart.contains(destinationPageCode)) {
    // some code with no obvious intent
} 
if (flightAvailabilityPages.contains(destinationPageCode)) {
    // some other code with no obvious intent either
} 

...which is expresses the intent much better. But...

Current Problem

The problem here is that if we add a page, we would in theory need to go through all the code base to find if we need to add our page to an if() or to a list like that.

Even if we moved all those lists to the PageCodes class as static constants, it still needs discipline from developers to check if their new page fits in any of those lists and add it accordingly.

New solution

My solution was to create an enum (because there is a finite well-known list of page codes) where each page contain some properties that we need to set:

public enum Page {
    FOFP(true, false),
    FOMS(true, false),
    BKGD(false, false),
    PURF(false, true),
    ITCO(false, true),
    // and so on

    private final boolean isAvailabilityPage;
    private final boolean hasShoppingCart;

    PageCode(boolean isAvailabilityPage, boolean hasShoppingCart) {
        // field initialization
    }

    // getters
}

Then the conditional code now looks like this (3rd form):

if (destinationPage.hasShoppingCart()) {
    // add some shopping-cart-related data to the response
}
if (destinationPage.isAvailabilityPage()) {
    // add some info related to flight availability
}

Which is very readable. In addition, if someone needs to add a page, he/she is forced to think about each boolean and whether this is true or false for his new page.

New Problem

One problem I see is that there will be maybe 10 booleans like this, which makes the constructor really big and it might be hard to get the declaration right when you add a page. Does anyone have a better solution?

Was it helpful?

Solution

You can take the idea one step further and define an enum for your page features instead of using booleans.

This makes it easy to add/remove a feature to a page and makes the page definitions instantly readable even when there are 30-40 potential features.

public enum PageFeature {
    AVAIL_PAGE,
    SHOPPING_CART;
}

public enum Page {
    FOFP(AVAIL_PAGE),
    FOMS(AVAIL_PAGE),
    BKGD(),
    PURF(SHOPPING_CART, AVAIL_PAGE),

    private final EnumSet<PageFeature> features;

    PageCode(PageFeature ... features) {
       this.features = EnumSet.copyOf(Arrays.asList(features));
    }

    public boolean hasFeature(PageFeature feature) {
       return features.contains(feature);
    }
 }
Licensed under: CC-BY-SA with attribution
scroll top