A few weeks ago I was about to write an acceptance test involving socket communication. Since I was only interested in a particular sequence of exchanged data, I needed to wait for the start command and ignore all information sent prior to that command. In this blog post I’d like to present the process of enhancing the readability of the tiny piece of code responsible for this task.
The first version, written without thinking much about readability looked something like the following:
private void waitForStartCommand(DataInputStream inputStream) {
String content = inputStream.readUTF();
while (!START_COMMAND.equals(content)) {
content = inputStream.readUTF();
}
}
The aspect that disturbed me most about this solution was calling inputStream.readUTF() twice (Remember: DRY). So I refactored and came up with:
private void waitForStartCommand(DataInputStream inputStream) {
String content = null;
do {
content = inputStream.readUTF();
} while (!START_COMMAND.equals(content)) {
}
In this version the need to declare and initialize a variable grants far too much meaning to an unimportant detail. So, a little refactoring resulted in the final version:
private void waitForStartCommand(DataInputStream inputStream) {
while (startCommandIsNotReadOn(inputStream)) {
continue;
}
}
private boolean startCommandIsNotReadOn(DataInputStream inputStream) {
return !START_COMMAND.equals(inputStream.readUTF());
}
This example shows pretty well how even rather simple code may need to be refactored several times in order to be highly readably and understandable. Especially code that handles more or less unimportant side aspects, should be as easily to understand as possible in order to avoid conveying the impression of being of major importance.
Schneide Blog 
Hello,
is the “continue” word in 3rd string used for clearness?
Thanks,
Anton
Correct. Leaving the loop empty or putting a
;after thewhile(...)left too much questions unanswered (imo). That would have been something like “while something does not happen”, instead of “while something does not happen, just continue waiting”, which is what I really want.Having to code another method seems to me less readable than
while(true)
{
if (inputStream.readUTF().equals(START_COMMAND)) break;
}
This emphasizes that I’mreading inputStream without limit until I get the result I want.
Actually, IMHO, I think that
while (startCommandIsNotReadOn(inputStream)) {
continue;
}
is more readable than
while(true)
{
if (inputStream.readUTF().equals(START_COMMAND)) break;
}
Calling the method is easier to read, explicit, removes the potential for missinterpretation, and allows me to ignore the details if I do not need to see them.
Hico, can you elaborate on the phrase:
“In this version the need to declare and initialize a variable grants far too much meaning to an unimportant detail.”
I am not sure I understand your reasoning behind the additional refactoring. Are you talking about reinitializing content with every loop?
Hello Sheldon,
actually, I’m referring to the very first line in the method: String content = null. This is nothing but a technical detail, required to check whether the awaited command was read, and its actual value is of little interest. Yet, to me this extra variable conveys the impression that the value is important (or at least more important than it actually is).
The simplest way to circumnavigate this is to inline the variable (which was, besides switching back to a while-loop, the first step towards the final version).
I hope this answers your question.
Looks good, except I wouldn’t use a negative in a method name (or any identifier really) if I can help it.
I think this would look clearer:
while (! startCommandIsReadOn(inputStream)) {
continue;
}
Using a negative identifier seems to require more brainprocessing for me when just skimming over code than seeing a language level negation.