What should my code tell me?

When I'm reading code, there are a number of things that make my job much easier.
The most important (in my opinion) is good naming, but how can we choose suitable names?
This is the pattern that I am starting to see (and recommend), and I think it is pretty effective.

What? How? Who? Why?

If we think of the code as a medium of communication, what questions should the code be able to answer?

What are we trying to achieve?
The method name should be a summary of what we are trying to achieve. For example, public void selectTheCustomerNamed(String customerName) {...}
It is (hopefully) clear from the method name what will happen when we call it, and what we need to pass in in order for the code to work.

How do we do this?
The body of the method should be the steps needed to achieve what it is we said we would do in the method name. The steps should be at the same level of abstraction. (See Composed Method)

public void selectTheCustomerNamed(String customerName) {
theUser.clicksOn(Application.SEARCH_BUTTON);
theUser.types(customerName,into(SearchPage.CUSTOMER_NAME));
theUser.clicks(SearchPage.BEGIN_SEARCH);
theUser.clicks(ResultsPage.FIRST_RESULT);
}


Who is doing the work?
In the example above, by naming the receiver of the messages after the actor who is likely to perform these steps, it helps focus the mind, and can help with appropriate method naming. For example, we can ask ourselves "What does theUser need to do next?"

Why are we doing this?
At one time or another, you will probably come across a comment like this:

// Add one to foo
foo++;

This kind of comment is irrelevant at best, it is telling us what the next line is doing. If the methods are well named, or the code is idiomatic, this is unnecessary. One negative consequence may be a kind of "comment blindness" where your mind filters the majority of comments as "noise".
A better use of comments is to explain why we are doing things in a certain way, especially if it seems unusual. Martin Fowler briefly mentions this in the "Code Smells" chapter in Refactoring.
An example of this type of comment is shown below. This represents a hard-won piece of knowledge, and will hopefully prevent a well-meaning programmer from replacing the code with the getFooByQuery variant without checking the caching behaviour.

// We query the id first before trying to retrieve the object, because if we try to
// retrieve the object directly (with getFooByQuery), it always bypasses
// the cache (see @link{...})
Integer fooId = findFooIdByQuery(query);
return getFooById(fooId);


JavaDoc can also be used to tell your users why they might need to pay special attention. If the code is well named, you are unlikely to need JavaDoc for all classes.
Here are some examples of where you might like to use JavaDoc:

• If you have a number of pluggable modules, why would you want to use this module


• Reasons why a user might want to provide their own implementation of this class


• Reasons why this code throws exceptions, and why a user might want to handle them

This is still a work in progress, but the results that I have seen so far have been encouraging. Please let me know in the comments if you have any suggestions for refinements or improvements.

Well, how did I get here?

And you may find yourself living in a shotgun shack
And you may find yourself in another part of the world
And you may find yourself behind the wheel of a large automobile
And you may find yourself in a beautiful house, with a beautiful wife
And you may ask yourself, "Well… how did I get here?"

Once in a Lifetime - Talking Heads

Logging a Stack Trace in log4j

A while ago, I was trying to replace a number of hard coded strings with an appropriate Enum. Unfortunately, the design of the code I was working on meant that it was difficult to find all the possible values for the strings.
I decided that the behaviour that I wanted was:

• If the string method was called with a new value, log that value, so that I can make a decision about whether I wish to add it to the Enum


• If the string method was called with a value that matched a value in the Enum, I wanted a stack trace, so I could replace that call with the appropriate Enum call

The code I ended up with was something like this:

public void doSomething(String theDestination) {
if(weKnowAbout(theDestination)) {
logger.debug("doSomething called with a known destination of " + theDestination, new Throwable());
}
else {
logger.debug("doSomething called with unknown destination of " + theDestination);
}
doWhatWeNeedToDoWith(theDestination);
}


Passing the new Throwable() into log4j creates a stack trace in the log output. I've found a number of places that tell you how to do this (now that I know how to do it), but it took me quite a bit of searching when I was first looking.

ILikeThisBlogPost

ILikeThisBlogPost - but then I'm a little biased :-)

Returning 'null' Considered Dishonest

Background

Antony Marcano and I have just started running a coding design workshop. Most of the audience are new to coding and we are trying to focus on good habits that are applicable across all programming languages.
In our first session, we created a vending machine. By the end of 90 minutes, it was able to dispense a drink as long as it had sufficient money and stock available.
One of the questions that we asked was "What do we do when the customer has not inserted enough money and we press the button for the drink?"
Some of the people who had some programming background said "Return null", which is what brings us to this post.

Good Citizen

In the wiki for the PicoContainer project, there is a page titled "Good Citizen", which details a number of good practices that all Java classes would benefit from trying to adhere to.
The practices make a lot of sense when you think about them, but they aren't really explained. I'm going to try and address that issue as we cover them in the workshop.
The practice that we are looking at today is "Never expect or return null", with a dash of "Fail Fast" for flavour.

What's so bad about null?

In the Java world, when we declare a method signature, we specify a return type. In the case of our vending machine we had:

public Drink giveMeADrink() {...}

By declaring that the return type is Drink, we are signing up to return something that "is-a" drink.
We could also return null, which is a nothing value. We could use this to represent that you did not get a drink.
The client code may look something like this:

myVendingMachine.giveMeADrink().downInOne();

If we return null, this code will fail with a NullPointerException. Not particularly useful, but at least we are using the result straight away. The problems become much worse if we store the returned Drink for use later.
When we said we will always return a Drink, we lied.

Programming Defensively

The sample client code above makes the assumption that the result of giveMeADrink will be a Drink. Given that we've actually signed up to that contract, that doesn't seem to be unreasonable. But now the client code is broken and they have an angry customer, they are going to have to work around the issue. It would probably look like this:

Drink myDrink = myVendingMachine.giveMeADrink();
if(myDrink != null) {
myDrink.downInOne();
}

This code is actually saying "I've asked you to give me a drink, but I don't trust you, so I will check first".

Why isn't this working? An Exceptional Approach

If we rely on our client to check that they received a valid result, we lose out on an opportunity to let the client know why the call was unsuccessful.
In the Programming Defensively example, we can recover from being returned a null, but we don't know why it was null. Was it because we hadn't inserted the money? Was it because the machine was out of stock? Was it because the stars were out of alignment?
Do we handle the scenarios differently? If we haven't inserted enough money, that's something we can deal with, but if the machine is empty, we need to look for another machine.

What if our code looked like this?

public Drink giveMeADrink() {
if(weDontHaveEnoughMoney()) {throw new NotEnoughMoneyException();}
if(weDontHaveEnoughStock()) {throw new DrinkOutOfStockException();}
return new Drink();
}

What we have said is "We will always give you a drink or tell you why we couldn't"

Now when we attempt to call giveMeADrink, it lets us know straight away if it can't proceed. It also gives us a good indication of why it is having problems.
The client code calls:

myVendingMachine.giveMeADrink().downInOne();

and gets told "Sorry, I'd give you a drink, but you didn't insert enough money".
Our code is being honest, polite and giving the client an opportunity to remedy the situation. The customer is still angry, but now he's angry with himself for not putting in enough money.

In Summary

• Programming defensively is programming distrustfully


• Returning null is dishonest. It requires others to check that we've upheld our side of the bargain


• Throwing a meaningful exception allows us to let the caller know why their call did not succeed