Coding tips #1: tell, don’t ask

As part of a new assignment, I have the privilege of working with a new team. Hey teamies! One of the things the team and I really want, is to make sure our code is clean and manageable. Hence, I’ll be putting up a coding tip once in a while on how you can improve your code for readability and maintainability.

Tell, don’t ask, is a reminder that you should endeavor to tell objects what you want them to do; do not ask them questions about their state, make a decision, and then tell them what to do (source).

In the LightSwitch world, entities are our models. All too often we are tempted to write code that asks the models about their state, then we take decisions about that state. For example:

if(currentCustomer.Orders.Any(c => c.ClosingCode.Code == "1209" || c.ClosingCode == "1304")
{
  //Preferred customer, add discount
  //More code here...
}

This works beautifully. It’s fast to write, any other developer can clearly read what’s going on, and it executes in 0ms flat (give or take a few ticks).

There’s a couple of issues with this type of coding though… First of all, how about readability? Can any developer clearly read what’s going on? Well, read: yes. Understand: no. If he/she does not know what these closing codes mean, he/she’s shit out of luck.  Secondly, there’s a maintenance issue. What if a closing code changes? What if a third closing code is added that makes a customer preferred, or one is removed? What if we want to support multiple tenants, and for one tenant closing code “1209” makes the customer preferred, but for another one it doesn’t?

In all of these likely cases, we’d have to loop over our entire code base and find all the spots where we ‘ask’ the object about its state, and then take actions accordingly, and update that code…

We can get a few quick wins in maintainability and readability by adding a new field to the ‘ClosingCode’ entity:

if(currentCustomer.Orders.Any(c => c.MakesCustomerPreferred)
{
  //Preferred customer, add discount
  //More code here...
}

It seems like just a small change, but adding this field to the ClosingCodes really improves readability (as a new developer, I don’t have to know about the actual codes), we can add/edit/delete closing codes that make a customer preferred, and we can support different closing codes for multiple tenants.

Almost there though: our code still has the hard-coded idea that a customer can only be preferred based on closing codes. What if one day we also want to make a customer preferred based on other parameters, like ‘simply being a nice person’?  This additional refactoring requires you to take a decision though: refactoring takes time, and time costs money. Are these scenarios likely to happen, or are they more of a ‘one day it’d be nice to have this’ (YAGNI – you ain’t gonna need it).

If there is a very real chance that these scenarios might happen, you can further reduce the complexity of the code by moving the ‘IsCustomerPreferred’ state into the Customer object, instead of keeping it hidden inside the OrderClosingCode table:

if(currentCustomer.IsPreferred)
{
  //Preferred customer, add discount
  //More code here...
}

Obviously, you’ll need additional code, for example when an order is closed, to tell the customer it is preferred or take that status away.

 

Keep rocking LS!

 

Jan

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s