Thursday, 26 January 2012

The code base is your company's most valuable asset: Part 1 implement peer coding standards

We have all been in presentations when the CEO talks about the how the staff are the most valuable asset the company has.   To be fair great employees and management empower companies to reach increased profitability and reduced cost.

One area I have always been surprised by is the lack of such attention / appreciation focused towards a company's software code base.  An asset is a resource with economic value which a company owns and is expected to provide future economic benefit.  In this blog I consider your software's code base as an asset and should meet these financial benefits.

In today's IT renaissance leveraging IT capability to increase productivity and provide a competitive advantage is at the core of Senior Management discussions.  There is many ideas of grandeur such as increasing sales through ecommerce channels, replace slow manual processes with automated ones and integrating systems with social media to get closer to your customer.   These all can offer real revenue benefit and they require development either through in-house or consulting development teams. 

To often most projects start with an ambitious idea under a strict deadline, this requires development teams and BA to get involved straight away with analysis to understand the requirements and also attempting to design a solution that meets business expectations.  Shortly afterwards development starts and its to often a high octane development cycle (hopefully agile) until the project completes.   No where in this project life cycle is the code base ever considered as a component which needs to be estimated, developed and maintained to ensure/enforce quality.  

Ignoring the value of code in your project implementation is a seriously flawed perception of project success even if the project goes live on time and in budget.  Not enabling mechanisms in your development team to ensure code quality will have significant and usually never understood or  quantified costs including
  • Most of the bugs end up in your UAT phase, these add costs to testing and development who need to fix these issues.
  • Delays project development as developers integrate poor and confusing class and API implementations meaning more development hours and usually overtime required to work around these poor coding practices
  • Production bugs become high risk, slower to fix and harder to implemented as simple changes can impact too many code areas in highly coupled and poorly implemented code.
  • New developers take longer to train and they are inexperienced trying to understand a poor code base.
  • A culture of poor code becomes the norm and acceptable so the longer the code base lives, your cost of maintaining it expediently increases.
  • Good developers who join are shocked by what they find and can be lost to poor standards around them.
  • The highest and never understood cost comes from future change, poor coding practices leading to poor solutions can make a business change that should only take 1 week turn into an 4 week development effort.  Imagine the cost of change 400% more than it should be, this is common.

A code base that is viewed as a valuable asset and is invested in will overcome the pitfalls highlighted above enabling your development team offer you competitive advantage in speed to market, cost and innovation. The characteristics of strong code base are
  • Efficiency - changes can be easily made and don't have wide functional impact
  • Training Cost - are reduced as developers can easily understand what the code is doing
  • No developer is bigger than the code base - To often you hear we can't let that developer leave and he is the only one who understands the code base (this is a key indication there is something wrong).  In a good code base developers can join an quickly start adding benefit through feature changes, thus removing the need for a super hero developer.
  • Quick handovers -A developer can easily hand over their work to other developers, thus reducing this hand over cost where no development is being done.
  • Cheap feature requests - Features requests can now be implemented at the most efficient cost.
  • Quicker to market - Your project and future change can get to market quicker as less time is spent in both development and test cycles.
In 2012 consider making your code base one of your priorities, understand what steps are being taken within the development teams to ensure the highest possible quality is met.  The leading software development teams are implementing the following practices
  • Peer code review
  • Test driven development
  • Automatic code verification and analysis (http://www.sonarsource.org/)
  • Continuous integration
  • Automated build life cycles
  • Automated regressions and acceptance testing
  • Project/Release retrospectives
To often companies consider the above list too expensive to implement, but they don't realise the real and higher cost is not implementing these practices.

Peer reviews

One the easiest ways to start improving your code base is peer reviews, a peer review should review both architecture/solutions decisions as well as good coding practices.  The set of programming rules outline below are promoted by the leading development gurus and are ones every team should stride to implement.  These rules are achievable in your team and are in a format which can be easily used by developers and evaluated against in the peer review.

This list is just a start and I will add more good practices over time.  Please review this whole article and take the rules from it which best suit your project and team.

Rule 1: Meaningful class, method and variable names

Ever heard people others talking about fluent code, you code should easily read what it does.   Be descriptive, I should get an idea of what the class, function or variable does/is from its name

 public class EmailValidator { 

This class is clearly labeled, if I was building an ecommerce app and needed validate email addresses I would search the JavaDocs for email and find this class straight away.

No more variables called i or thePerson or person.

Rule 2: Methods should be small

Remember a function should only do one thing, I think setting a single rule like a method should not have more than 7 lines is a great way to promote small single action methods.  A hard rule of 7 lines cant always be accepted by developers.  As a max rule your should never need to scroll your IDE to view the whole function.

Rule 3: Number of method arguments

Again their should be a limited number of arguments per function, too many arguments point to a function doing too much.
  • 0 - Great
  • 1 - Good
  • 2 - Ok
  • > 2 should never be done

Rule 4: Always use return methods

Try to avoid using void, returning an object enforces your aim to alter the object.

Rule 5: Never use flag arguments

Never use a flag as a parameter, you should be able to derive the flag logic from another parameter.

Rule 6: Methods should either do something or answer something

Methods should only
* Do something
* Answer something
Never both

Rule 7: Methods should have no side effects

No unexpected behavior outside the their scope of the function.

Rule 8: Don't ever repeat yourself in your code

If you are copying code ask yourself why, its wrong.  Simply don't do it.  This should be monitored in your code base by using the duplicate code analysis on Sonar.

Rule 9: Extract exception handling

Move handling the exception handling from the logic, meaning the logic after the try should be in its own function.  Makes the code easier to understand and the function is doing too much.

Rule 10: Method A in Person class should only call

  • Method in class person
  • An object created in that method 
  • An object passed in as a parameter
  • An object instance of person

Rule 11: Every if condition should evaluate to a function

Every logic evaluation should call a function returning a Boolean.  This will make the goal of the logic much clearer.

eg

f(customer.hasAuthorisedEmailCommunication()) {
        sendEmail(customer);
}

I have seen so many legacy if statements in code where I did not have a clue what they were being used for, using a function for evaluation immediately spells out the business intent.

Rule 13 Don' t comment your code

If you implement the last rules you should not need to comment your code.  This is the challenge to you and your peers.  If code is written cleanly and easy to read then comments are unnecessary.  This can easily be reviewed during peer reviews.  If there reviewer cannot understand you code in one pass over it needs to be refactored .  

Rule 14 Data transfer objects

Rarely have I seen these implemented properly, should set all variables in as constructor, or you can use the builder pattern and all variables are available through getters.  No setters in these classes please.

Rule 15 No train wrecks please

eg. customer.getAddress().getPostCode().getCity()

This is always a shocker when I see it, functions should not be grouped on top of each other.

Rule 16 Prefer exceptions

Don't use error codes

Rule 17 Throw exception

Throw exceptions instead of try / catch.  To often exceptions are catch and hidden too early.  When you are wrapping an exception in try catch ask yourself is this the last time you will need this information.

Rule 18 Write tests  to force exceptions

When writing tests ensure you test the exception scenarios as well.

Rule 19 Always provide context with exceptions

Every error message should contain context like an id, name.  Provide some unique information so the error can be found and understood in the logs.  Log errors without context are USELESS.  I have spent (and probably wasted) too much of my life searching through poor logs

Rule 20 Don't ever, ever return NULL

How often do we as developers check for NULL, time to change the game where NullPointerExceptions are real unexpected errors.  First step never return null.

Rule 21 Don't ever allow NULL to be passed

First step was never to return null, second step never expect a null parameter 

Rule 22 Watch passing maps in an API

If you are using maps in your API, wrap the map implementation.  The reason for this is other users of the API can remove objects from the map, thus changing the state unexpectedly.

Rule 23 Maintain correct state in your class

Use the constructor properly, any object that is mandatory for valid state of you class should be included as a constructor element.  I too frequently see hibernates SessionFactory being passed into DAO classes via a setter.  What DAO object is valid without access to its datasource? 

Rule 24 Never require more than one method call on a class to achieve desired functionality

This is definitely linked to the last rule, as constructors are often not used correctly it promotes the confusing implementation, where more than one function on a class needs to be called to achieved desired behavior.  This is great when the code is being written, but future developers will not understand or know about this multiple method calls to achieved desired output, so will lead to future confusion and bugs.  This is simply shocking and should not be done.

Rule 25 Use Global Variables and Constructors Correctly

I often see in code developers not sure when to initialise the a global variable or to instantiate it to null.  Follow these rules and its easy

If the variable is instantiated in every constructor then do not instantiate it.  Ideally no object reference should be null

private ValidatorEnum nameValidator;
private String name;
 
public SampleClass(ValidatorEnum nameValidator, String name) {
 this.nameValidator = nameValidator;


Rule 26 Unit Test Leap Year For Every Date method

I seen an issue recently where some code did a calculation, adding a year to a current date, the way the code was written it simply did not work for leap years. Shocking, simply force unit testing to evaluate against leap year, often missed in user acceptance and has the potential to take systems offline once every 4 years.