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 if
s 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?