Motivation for one-line methods

Posted by Chris on April 24, 2008

A simple thing that I often do to clean up and improve code as I am trying to understand it is to extract short fragments of a method into a new, smaller method. Quite often I will move just a single line of code into a new method. The reason for doing this is of course to document why the line exists by naming the method accordingly. However, even though more and more people are now realizing that comments are a code smell, I am sometimes challenged to provide a motivation for extracting it into a method instead of simply writing a comment for that one line of code.

Take a look at the code fragment below:

	public void readSomeData(int x, int y) {
		int position = x / y - this.z;

		startReadingFromPosition(position);
	}

That first line of code calculates a value according to some rules that are not quite clear from just seeing it. Lets say that this specific calculation is called the UltraMegaAlgorithm. Naturally we need to document this to help readers know that we use this algorithm. But why not simply write a comment? What is the difference between the two alternatives below?

	public void readSomeData(int x, int y) {

		// Use UltraMegaAlgorithm to calculate starting position
		int position = x / y - this.z;

		startReadingFromPosition(position);

	}

	/* -------------------- */

	public void readSomeData(int x, int y) {

		int position =
			calculateStartingPositionUsingUltraMegaAlgorithm(x, y);

		startReadingFromPosition(position);

	}

	private int calculateStartingPositionUsingUltraMegaAlgorithm(
		int x, int y) {

		return x / y - this.z;

	}

Some will argue that it is much easier to read the comment than the camelCased method name. And I agree, it is really unfortunate that method names are often written this way (I think Ruby’s calculate_starting_position_using_ultra_mega_algorithm is at least better, but it is difficult to get Java and .Net programmers to change their old policies). But I would still argue that the second alternative is better. I could argue that we might want to do more than simply extracting into a new method. Perhaps we want to Move Method into another class. Perhaps this might become a strategy implementation. But I think there is a much simpler reason.

My main motivation for extracting into a method lies in considering what is important to us when we read the code to try and understand it. We want to see that readSomeData uses the UltraMegaAlgorithm to calculate a starting position, and then starts (doing whatever it does) from that position. We are completely oblivious to how the UltraMegaAlgorithm is implemented! The information in the comment is what is interesting to us here, not the implementation following it. In fact, that line makes it harder to understand what readSomeData does, since we will stop to consider the implementation even if we do not want to.

My rule of thumb is that the steps of a non-private method should be easy to understand by simply glancing at it. The way to achieve this is to make it read like normal language, which means that unclear one-liners with comments should be replaced with a method call that hides the implementation that the comment describes the reason for. If I trust the implementation of calculateStartingPositionUsingUltraMegaAlgorithm to be correct I can decide for myself if I just need to know that the code does this, of if I want to drill down and learn about the implementation as well.

Trackbacks

Trackbacks are closed.

blog comments powered by Disqus