Thursday, 14 June 2012

Can you read your own code?

I have been reading a lot lately into how to produce better quality and code of a more professional standard.

A lot of focus is put on the readability of the code especially to future developers who may be performing maintainence.

I was recently working on a project written in grails and specifically on a service called TranslationService. This is a fairly simple service which wraps a call to the internal grails messageSource object. Within this service we resolve a locale from a locale resolver that we have written and pass this through to the messageSource object.

We wanted to change this to allow us to pass through a context and from this context retrieve a locale resolver from a map while defaulting to the original locale resolver we had defined as a property of the service. So while pair programming with a grails and groovy newbie we began.

The original line of code before it was changed is shown below.

"def locale = localeResolver.resolveLocale(request)"

The following line of code is the result of our efforts.

"def locale = ((localeResolverMap ?: [:])[context] ?: localeResolver).resolveLocale(request)"

We primarily develop using a TDD method and our initial effort had most of the logic being performed in another private method on the service. With tests written and the functionality correct we began to re-factor and the line of code above is the result.

At first the final result was very pleasing to the eye. From a method which initially was about 5 - 10 lines in length we had re-factored to one line. Knowing it worked it has been deployed. Since then I have been thinking about the readability of this line.

We called over an experienced grails/groovy colleague and asked him to try and explain without any context what the line of code did. While he did work it out it was not immediately obvious.

Effectively we have a line of code that while pleasing to the original author and ultimately readable by experienced developers it would be slightly more difficult to a novice developer or someone trying to make a quick fix a year from now.

So what are the options?
  • We could leave it as it is and to hell with the future developers. If they are not skilled enough to work out what the code is doing then they should not work on the project itself.
  • We could comment the line of code to explain what we are trying to achieve.
  • We could factor the line of code out into a method and give it a clear descriptive name.
  • Where a clear and descriptive name is not available or feasible we could factor out into a method and comment the method itself.

Each of these options can be performed without too much work but which do we go with?

The first option of leaving it as is is not the best approach. While the possibility of making another developers job harder it has the possibility of myself returning to this code in a year or two and having to work it out myself. I for one am all in favour of making my life easier. In this instance the code is not too complex and people who can program in groovy should be able to work it out, but I am using this example to make a wider point. So for arguments sake lets say this option is out.

Commenting single lines of code is out too. I don't like this approach. While I agree comments should be used, I think they should explain a class or method and not individual lines of code.

Factoring code out to a single method with a clear and descriptive name. Sounds perfect. But what is that line of code doing? In essence it resolves the locale. But it does more than that. It gets the locale resolver based on a context and from a map if that context is a key in the map or the default locale resolver. Not as easy to get that in a method name that is clear and concise.

So we are left with a method with a clear and concise name and a comment on that method. This ties in nicely. We still get to keep out nice, complex and tested one line of code while still allowing all future developers an easy job of working out what it does.

At the end of the day the code we write is an evolving entity, and making the job of developers extending, modifying or improving this code easier is something we need to be thinking about very seriously. All too often do we hear everyone in the office complaining about the code they are having to read through. So from now on I am going to make a real conscious effort to improve the readability of mine.

No comments: