Agile. Kanban. Lean Startup. We have more and more ways to think about the software development process, and our industry is getting better and better at what we do. We’re wasting less time, and delivering a more customer-focused product. New businesses have great tools to build great teams. We know how to manage source code, and some people have even worked out how to manage changing data models! Issue tracking is, by and large, a solved problem, and we have tools to keep our software fully tested and behaving the way it should.

What I don’t see enough focus on these days is the code we write. Sure, we’re all doing peer reviews (right?) and we think we manage code quality through unit testing. What worries me is that, usually, this just ensures that our code works. It does nothing to really manage quality. What’s wrong with this code?

class CreditCard
{
  private Decimal _Limit;
  //...
  public bool RaiseLimit(Decimal amount)
  {
    _Limit += amount;
    //....
  }
  //...
}

This is a blatant example, but it’s a problem I see all the time. The code looks clear enough, and it’s going to work. It will build, pass your unit tests, and there’s every chance it will pass peer review – but it shouldn’t. The problem is, there is a risk that someone will do this:

Decimal oldLimit = card.Limit;
Decimal newLimit = ...;    // Code to calculate the new limit
card.RaiseLimit(newLimit);

… and that would be awful! The caller has gone to the effort of calculating the new limit and passing it to RaiseLimit(), but that’s not what RaiseLimit() expects: it expects to be passed the amount the limit should be increased by. How do we solve this? There is a simple change we can make to reduce the risk this will happen. Our improved code might look like this:

public bool RaiseLimit(Decimal increaseAmount)
{
  _Limit += increaseAmount;
  //....
}

Functionally, nothing has changed – but we’ve provided an extra clue to a developer calling our function. In these days of intelligent IDEs, coders usually get to see the method signature that they’re calling. We should endeavour to take every possible advantage of IDE features like these to streamline code creation and reduce the chance of errors.

This is one simple example from a whole spectrum of things you can do to optimise code-writing. I have wasted countless hours reading through classes trying to work out how to use them. Here’s a recent example I ran into:

class ConfiguredTask
{
  public string ConfigFolder;
  public string TaskExe;
  public void WriteMainConfig(DateRange dates, string configName) {...}
  public void WriteSecondaryConfig(DateRange dates, string configName) {...}
  public void RunTask(string path, DateRange dates) {...}
  public ResultSet GetResults(DateRange dates) {...}
  //....
}

To use this effectively, you need to know quite a bit about the internal class. For a start, you need to know that you need to write the main and secondary config files before calling RunTask() – not as obvious as you might think, as there are dozens of other public methods and properties on this class. Second, you need to know that the two functions to write config files are expecting filenames with full path information, and they need to be different, but in the same folder. Third, you need to know that RunTask() persists the results of the task both into the database – something I didn’t want – and leaves output in the folder referenced by ConfigFolder. TaskExe must contain the name of the file to run, but must not contain any path – the executable must be in the path referenced by ConfigFolder. That wasn’t even the end of it! The code to run a task used to look like this:

task.WriteMainConfig(selectedDates, @"c:\main.cfg");
task.WriteSecondaryConfig(selectedDates, @"c:\second.cfg");
task.RunTask(task.ConfigFolder, selectedDates);
ResultSet results = task.GetResults(selectedDates);

Keep in mind that, if you didn’t have an example to hand, you had to read a fair portion of the class itself to make sure you had everything right! After I was finished with it, the class looked like this:

class ConfiguredTask
{
  public string PathToExecutable;
  public string ResultsFolder;
  private void WriteMainConfig(DateRange dates) {...}
  private void WriteSecondaryConfig(DateRange dates) {...}
  public ResultSet RunTask(DateRange dates,
    bool persistResults=true, string WorkingFolder=null) {...}
  public ResultSet GetResults(DateRange dates) {...}
  //...
}

Just through a little re-factoring and a handful of code tweaks, the caller now knows to give a full path to the executable (which can now be anywhere), they don’t need to know about writing config files (it’s done during RunTask()), they know that results are persisted by default but they can override that behaviour, they know they can provide an optional working folder, and they don’t have to call GetResults() after every call to RunTask() when they want the results of that task.

I’ve taken a class which needed quite a bit of reading to work out how to use it, and turned it into a class you can use with no more information than what IntelliSense shows you as you code. The code to run a task and get the results now looks like this, instead:

ResultSet results = task.RunTask(selectedDates);

I hope you can see why this would save a future developer time! We should strive for all of our classes to be like this. The idea of ‘self-documenting code’ doesn’t quite capture this: the whole point is to not have to read the code at all. I prefer the term ‘Intuitive Interface’. As a developer, you should aim for all of your classes to have this kind of intuitive interface. Think of the questions a caller might need answered, and then answer them – in your method signature if possible, and if not, in a method comment (ideally, in a fashion that leverages the IDE you’re working in):

/// <summary>
/// If the instrument is not running, set the passed-in mode and start it running.
/// If the instrument is already running it will *NOT* change the mode.
/// </summary>
private void EnsureInstrumentIsRunning(InstrumentMode mode)
{
  // ...
}

Even in that example (which I took from a current project), the first line is really extraneous – it’s repeating information that’s already available in the method signature.