Search This Blog

Monday, May 17, 2010

Train Wreck Code

I read something funny today in Growing Object-Oriented Software Guided By Tests. They were explaining the Law of Demeter (Tell Don't Ask) and they called it train wreck code. For example they showed this code


((EditSaveCustomizer) master.getModelisable()
.getDockablePanel()
.getCustomizer())
.getSaveItem().setEnabled(Boolean.FALSE.booleanValue());


They called it train wreck because the getters are chained together like the carriages in a train. I have never heard that before but I thought it was pretty funny.

They recommended changing the code to something like this


master.allowSavingOfCustomisations();


They made a good point about the method name. They said it should say something about "what the method is for" not just "how it is currently implemented".

I ran into an example of this the other day. We trying to make a conditional more expressive so we changed this code


if (property !== "query")
{
key.addProperty(property);
}


to this


var isNotQuery = property !== "query";
if (isNotQuery)
{
key.addProperty(property);
}


The explaining variable what was put in to try and explain why we needed the conditional. When we looked at the code though it didn't really add much so we changed it to this


var isNotExtReservedWord = property !== "query";
if (isNotExtReservedWord)
{
key.addProperty(property);
}


Now you can see that "query" is a reserved word so it was filtered out.

1 comment:

  1. I was reminded of this situation when reading clean code this weekend. He talked about naming your variables not just to describe what it is but to show it's intent. I like the term "train wreck code".

    ReplyDelete