The other day I had a nice conversation about naming of classes and methods which reminded me about one of the techniques I recently showed to a friend of mine.
He was having trouble with the unit tests he was writing (in Java) as he was concerned with the readability of them. He knew that I am more into Ruby these days and test my code using RSpec which leads to very readable test code so he showed me his tests and we started talking about them and how to make them more readable, i.e. treat them as production code as it should be.
I come form a school of thought where I believe that code should be read like a book; the domain coded being as close to the real domain in terms of the language used. Let’s look at the code in a step by step basis.
The original test looked something like this:
1 2 3 4 5 6 7 8 9 10 | public class SessionControllerTest { //... @Test public void testSendResetMessage() { final SessionReset reset = new SessionReset(); final Message<?> message = MessageBuilder.withPayload(reset).build(); sessionController.resetSession(); assertEquals(message.getPayload(), gateway.message.getPayload()); } //... |
Now form a syntactical point of view this is striking to me. Let me read the test aloud:
A session controller
Testtest send reset message equals the payload of the payload of the message in the gateway…
What did I just say? I have actually no idea what I meant here…
The biggest problem is the way I try reading the class in question. As you have seen I deleted the Test word of the class name while reading. I usually do that in my head when I read test classes because I have the feeling that it makes no syntactical sense when I read.
Then I really don’t like this line:
3 4 5 6 | @Test
public void testSendResetMessage() {
//...
} |
It’s just to verbose and doesn’t bring to much value to the readability of the test in question. What can be done though is moving the needed keywords and annotations away form the name of the method itself:
3 4 5 6 | @Test public void testSendResetMessage() { //.. } |
This looks a bit better, but the method name still has the same problem when trying to read the whole thing aloud. So let’s change that and get some nice naming in there so that we can read this (or even better, so that our customer can read it and tell us if she likes it and if it reflects what she expects!):
3 4 5 6 | @Test public void shouldSendAResetMessageWhenTheSessionIsReset() { //.. } |
Reading this aloud produces a much more cohesive language (we could argue that the method name is to long, etc, etc, but actually I think it improves the readability of the program, which is more important):
A session controller
Testshould send a reset message when the session is reset.
As you can see I have even omitted the explanation of that it should do (… the payload of the payload of the message in the gateway…) as it is not longer needed for the understanding of what is going on inside the method any more.
Just as an overview here is the complete class with the new should notation:
1 2 3 4 5 6 7 8 9 10 | public class SessionControllerTest { //... @Test public void shouldSendAResetMessageWhenTheSessionIsReset() { final SessionReset reset = new SessionReset(); final Message<?> message = MessageBuilder.withPayload(reset).build(); sessionController.resetSession(); assertEquals(message.getPayload(), gateway.message.getPayload()); } //... |
My advice in general would be to write code so that our customers can read it. It doesn’t only help the customer, but makes it possible for other fellow developers and craftsmen to understand and follow our code in an easier way.
Cheers!
Leave a Reply