Skip to content

“Clean Code Gems”

April 30, 2014

Having read Robert C. Martin “Clean Code – A Handbook of Agile Software Craftsmanship” back in 2010 at the time I didn’t really appreciate much of the gems raised and I skimmed through much of the book. At first glance my initial thoughts were that this book just had some well known common sense basic programming practices that every newbie would know. To a certain extent this was true but

Many years on I am back to this book; this time having read it cover to cover I realised how beautifully clear and simple these basic programming concepts are portrayed. Although what is said in essence is general coding practices that we all love and adhere to; having these written down in such a clear manner is very useful. The book has become one of my favorites and I now recommend this book to every programmer.

Below I’ve listed some of the simple rules that one should aspire to while writing code.

  • Method & classes should have meaningful names.
  • Methods should be SMALL and do ONE thing ONLY.
  • Switch statements should be avoided unless absolutely necessarily and if used they should be buried deep in the code and never repeated.
  • Function arguments the less the better; the best functions are those with clear names and no arguments at all.
  • Return exceptions rather than error codes.
  • Comments are necessary evil … use only if it’s a legal requirement/warning/must have amplification/TODO comments and public API comments.
  • Separate business logic from error handling.
  • Methods should NOT return null.
  • Obey the three laws of TDD.
  • Classes should have single responsibility as with functions.
  • Maintaining cohesion results in many small classes
  • Concurrency: limit scope of updated data
  • Use thread safe collections
  • Default constructor no use
  • Avoid feature envy classes
  • If order of functions is important, temporal coupling should be enforced.

… examples of bad code to go along with it.

Final Note

This book is well worth a read. For a new developer it’s an absolute must. For experienced developers it servers as a good refresher of the how code should be crafted. I know by obeying those gems above has helped me greatly.

For further evidence read this post: (Why clean code is more important than efficient code)

p. 17 – Method & classes should have meaningful names …

Never under estimate the importance of giving your methods and classes meaningful names.

An example of recent code that I came across along my journey where this simple rule was badly violated is a method called ‘purnDidYouMeanWords’.

protected String purnDidYouMeanWords(SearchCriteria criteria, List assets, List<String> didYouMeanWords) {

….

}

Ignoring the clear spelling mistake and the missing generic type; in this context what does the word prune mean? How does it tell you what this method is actually doing? And why are assets passed in? What has that got to do with pruning words?

Without looking inside this method from the method name you cannot be sure of what it will do …

p. 35 – Methods should be SMALL and do ONE thing ONLY.

This is very important rule but many developers just choose to ignore it ro are just too lazy and stuff just too much into a method creating unreadable code, causing maintenance headache and potentially horrible side-effects. Looking at the purnDidYouMeanWords again we see it actually does not more than what it says on the tin.

Methods should be small and do one thing ONLY

Methods should be small and do one thing ONLY

 

 

 

 

 

 

 

Here we are doing several things; count with me:

  • checking if search text is digits

  • Then we are changing the search criteria if there is “did you mean words”; possibly creating side effects

  • After we try to find results with the new criteria; so we are in effect doing a NEW search.

  • Finally worst of all we are actually ADDING to the list of assets and definitely.

Now what has all of these has got to do with pruning “did you mean words”

If you see anything like that in the along your travels write a unit tests and REFACTOR.

By having small method; the rule “DO ONE THING ONLY” instantly become easier to apply test/debug.

p. 37 – Switch statements should be avoided unless absolutely necessarily and if used they should be buried deep in the code and never repeated.

Switch statment should be buird deep

Bury this deep in code … otherwise trouble.

 

 

 

 

 

 

 

 

What happens if next we are asked to add a isPayday() method. We will have to create the same structure to deal with this new method … and possibly other future methods.

The solution to this problem is to bury the switch statement to low level code.

Failur to bury switch statmes

Creating the same structure over & over is not a good idea …

 

 

 

 

 

 

 

 

 

Note: even though we increased the number classes and added an interface ‘EmployeeFactory’ we have actually made the code much more extensible.

An even better solution would be to use the strategy pattern and just use pass the correct object at runtime to call methods like calculatePay(), isPayday() etc. but that’s for another discussion.”

p. 40 – Function arguments the less the better; the best functions are those with clear names and no arguments at all.

If a method has more than four arguments then surly it’s doing more than one thing; look out for those.

Tip: Also if you see a boolean argument to a method; be very suspicious as it’s likely to be doing more than one thing.

p. 46 – Return exceptions rather than error codes

I once fell into this trap once when I thought it would be quick and easy to return an int error code. In the end the code got so messy that I had to re-write the entire thing.

p. 53 – Comments are necessary evil

Use only if:  1. It’s a legal requirement 2. warning amplification 3. TODO comments 4. Public API comments.

Code changes very often and one of the most confusing things are old comments that don’t have any meaning.

When developers changes code in my experience they don’t usually go through the class and see if the comments are still relevant. Be honest do you do that?

Here is one I recently came across:

// ‘shippingCompleted’ will be set to true if we pay by paypal. After

// the pipeline is ran this is called from JS.

Although this comment seems harmless; in fact you might argue that it’s useful; well actually there is no reference to PayPal in this entire class. As it turned out the PayPal code was completely removed from this class making this comment redundant and confusing.

p. 109 – Separate business logic from error handling

Separate business logic from error handling

Separate business logic from error handling

 

 

 

 

In the above example the ‘getTotalAsString’ method should be modified to return ‘0’ instead of throwing an exception. This way we can write code like:

Error handling should be kept as close as possible to where the error could occur; in this case it’s inside ‘getTotalAsString()’.

p. 110 – Methods should NOT return null

This is a pet hate of mine; it just clutters your code with  if (x != null) statements making the harder to read. With regards to lists ALWAYS return an empty list. Look at the null object pattern if in doubt.

p. 122 – Obey the three laws of TDD

1. You can’t write any production code until you have first written a failing unit test.
2. You can’t write more of a unit test than is sufficient to fail, and not compiling is failing.
3. You can’t write more production code than is sufficient to pass the currently failing unit test.

[Source: The Three Laws of Test-Driven Development]

Test Driven Development is a whole world in itself; but speaking from experience project that I have done with TDD framework were the most rewarding but it’s hard to get the initial infrastructure right.

p. 138 – Classes should have single responsibility as with functions

Classes should do ONE thing and ONE thing ONLY!!!

But I will have so many classes

Seriously a few lines of code to improve readability; will be well worth it in the end; don’t forget most of our time as developers is spent reading code and not writing; so having a few extra classes isn’t going to hurt.

p. 140 – 141 – Maintaining cohesion results in many small classes

This comes back to the point of classes doing one thing and one thing only.

p.181 – Concurrency: limit scope of updated data

p.182 – Use thread safe collections

I haven’t worked with a lot of threading code; but these rules makes sense.

p.293 – Default constructor no use

Joshua Bloch Effective Java might have different opinion on this one but if it adds no value then there is no reason to have. Keep code clean.

p.293 – Avoid feature envy classes

p.302 – If order of functions is important, temporal coupling should be enforced.

Or think of design patterns that can enforce this; e.g. template pattern.

Hope you enjoyed this post !!!

Advertisements

From → Uncategorized

Leave a Comment

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

%d bloggers like this: