rc3.org

Strong opinions, weakly held

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.

4 Comments

  1. Huh… can’t you return like this in Java?

    return (cart.getItems().size() >= CART_SIZE_FOR_FREE_SHIPPING);
  2. Yeah, that’s probably a better way to write that method.

  3. I think it would be better to have code in the addItem method of Cart that automatically sets the shipping to 0 when the new size of the cart exceeds the constant. And then have code in removeItem that checks if the threshold is reached after removing an item and recalculating shipping if shipping is no longer free.

    Or, have code in the getShippingCost method that returns 0 when the cart size exceeds the constant, otherwise returns the correct cost.

    In any case you can still have the boolean accessor that the controller can use to realize that there is an automatic free-shipping situation.

    The controller layer shouldn’t be responsible for setting the shipping cost based on a business logic rule, that’s the responsibility of the domain layer.

  4. I agree with that as well.

Leave a Reply

Your email address will not be published.

*

© 2016 rc3.org

Theme by Anders NorenUp ↑