Obvious programming tip for the day

Any time you have an expression in your code, it answers a question. For example, you might have an expression like this one:

if (cart.getItems().size() >= 5) { cart.setShipping(0); }

A lot of times you’ll see code like that in a servlet, or in a controller if you’re using an MVC framework. What I’ve realized is that such code should always live in whatever class “cart” is, and the number 5 should be stored in a constant somewhere.

So in Cart.java you might have:

public static final int CART_SIZE_FOR_FREE_SHIPPING = 5;

and a method like this:

public boolean isEligibleForFreeShipping() { if (cart.getItems().size() >= CART_SIZE_FOR_FREE_SHIPPING) { return true; } else { return false; } }

Then back in your controller, you’d have a snippet of code like this:

if (cart.isEligibleForFreeShipping()) { cart.setShipping(0); }

Why go to so much work for one tiny little expression? There are two reasons, both associated with DRY. First of all, that 5 is an important piece of information. You may have expressions containing the number 5 throughout your code, with various meanings. If you want to lower that to 3, or raise it to 7, doing a search and replace on your code isn’t going to work. That value needs to live in one place, and a constant is as good a place as any. (If you want to change it on the fly you’ll need to extract it from your code and store it in a database, of course.)

Moving up a level, you only want the business logic that determines which orders are eligible for free shipping to live in one place. Maybe next week you want to change things up so that orders of over $100 are eligible for free shipping. Maybe you want to check on the shipping costs whenever an item is added to the cart, whenever an item is removed, and when the order is being processed. Keeping the “free shipping” expression in a method in the class about which the question will be asked is most likely the best spot.

Another benefit is that the code in the later example is more readable after refactoring than before. The expression is not self documenting, a method name like isEligibleForFreeShipping is.

I used to think of DRY in terms avoiding the use of copy and paste, but I think there’s even more value in making sure that business logic is never duplicated. Making sure that all of your business logic, even if it’s just one constant or a simple expression, exists in only one place in your application and writing proper unit tests for it prevents a lot of bugs of the most annoying kind.

The real trick is to make sure that you don’t lazily duplicate the logic yourself later on rather than remembering to use your convenience methods. I haven’t figured out the answer to that one yet.