Single exit point

activity
initial post: 06 may 2011
last update: 11 may 2011


At the beginning of my career I was lucky to be guided to learn object oriented principles and patterns. At that time I was more concerned in learning frameworks. The colleague who guided me was passionate about XP and the related principles and practices, so I started to read what he read: Martin Fowler and Robert Cecil Martin.

One thing I learned from Fowler’s Refactoring book was to use “Guard Clauses”: to use if statements at the beginning of the method that checks for conditions that does not permit to follow the normal course of the method. So now I prefer to use an if with a return in his body instead of putting the rest of the method’s body into the else block.

For example:



Instead of:



Or:



* This is not a “SIDE-EFFECT-FREE FUNCTION” (see DDD - Eric Evans) nor the same thing with another name: “Separate Query from Modifier” refactor from Fowler. Or more generally: it breaks SRP principle or, it has a low cohesion, but this is another discussion. The name of the method suggests that it is a query but it also modifying state.


Instead of let's say:



In my opinion, the first version is more easy to read. But as I found later, there are some other opinions too.

The first divergence in seeing this as a good thing, I found it by using PMD tool. In PMD is a rule “OnlyOneReturn”, that is turned on by default. That rule complains about having multiple return statements into a single method. I found soon that this rule is on a “Controversial Rules” list on their site. OK, I turned off that rule and I didn't investigate the root of the controversy in opinions.

Later, by reading the Uncle Bob’s book: “Clean Code”, I found the reason why other people think that a method should have only one exit point: “Structured programming” techniques.

Here is what Uncle Bob said about that:

Structured Programming
Some programmers follow Edsger Dijkstra’s rules of structured programming. Dijkstra said that every function, and every block within a function, should have one entry and one exit. Following these rules means that there should only be one return statement in a function, no break or continue statements in a loop, and never, ever, any goto statements.

While we are sympathetic to the goals and disciplines of structured programming, those rules serve little benefit when functions are very small. It is only in larger functions that such rules provide significant benefit.

So if you keep your functions small, then the occasional multiple return, break, or continue statement does no harm and can sometimes even be more expressive than the single- entry, single-exit rule. On the other hand, goto only makes sense in large functions, so it should be avoided.


Another bad thing it is that the rule introduce control flags in code:


Such control flags are more trouble than they are worth. They come from rules of structured programming that call for routines with one entry and one exit point. I agree with (and modern languages enforce) one entry point, but the one exit point rule leads you to very convoluted conditionals with these awkward flags in the code. This is why languages have break and continue statements to get out of a complex conditional. It is often surprising what you can do when you get rid of a control flag. The real purpose of the conditional becomes so much more clear.

…........................

I often find I use Replace Nested Conditional with Guard Clauses when I'm working with a programmer who has been taught to have only one entry point and one exit point from a method. One entry point is enforced by modern languages, and one exit point is really not a useful rule. Clarity is the key principle: if the method is clearer with one exit point, use one exit point; otherwise don't.

….............................

Nested conditional code often is written by programmers who are taught to have one exit point from a method. I've found that is a too simplistic rule. When I have no further interest in a method, I signal my lack of interest by getting out. Directing the reader to look at an empty else block only gets in the way of comprehension.
Martin Fowler - Refactoring


One goal of structured programming is to make programs easier to understand. But single exit point is valid in a different context then we have today. It is funny that a rule that was introduced to make code clearer, now, applied in the wrong context, by unskilled developers, it makes it harder to read.

I like how a guy in a presentation on InfoQ pointed this out (and naming it as zombie practice):



[minute 03:44]
There are still people around you that are very keen about structured programming. This are zombie practices that we should do away with them. And the reason why we should do away with them is that they no longer apply in the context that we have today. ….. There is a context within we should do our work and we should choose practices appropriately for that context.
[minute 22:02]
Structured programming is one of zombies that use to make sense in the days when you have to make your own control structures every time. Like in Fortran. I did lots of Fortran in the university, whith lots of this kind of stuff where you need to roll your own looping as gotos and labels. In that context structured programming makes a lot of sense. In the world of the curly bracket languages.... no, no no no no no. Single entry and exit points? no, no no no no no. Is not Fortran. That stuff is not appropriate. It makes the code more complex, there are better ways of managing that kind of stuff.


Now, back to my story that made me to write this post. Last year I joined a new created team. Been a new team, we have to establish a set of coding guidelines. One team leader already had such a set of coding good practices from the last job. So we started from that document. From that list of guidelines I strongly disagreed with a particular one:
“It is recommended to use one return per method, in order to make clear the flow and the exit points of a method.”

So I a meeting I presented the disadvantages of introducing such a practice and I endorsed my opinion to remove that “good” practice from the document.

Then a debate started around what object oriented code means. The team leader, with a lot of effective working experience in java, pointed out that object oriented means to keep the code together to be easier to follow the behavior. Then he affirmed that once it took it several hours to find the place where to modify some behavior on the code written by me.
In that team I didn’t manage to convince anybody that the leader they follow write procedural code inside the service layer and is using an anemic domain model.

This incident reminded me a phrase what I’ve read some years ago:

In general the OO style is to use a lot of little objects with a lot of little methods that give us a lot of plug points for overriding and variation. This style is very confusing to people used to long procedures; indeed, this change is the heart of the paradigm shift of object orientation. It's something that's very difficult to teach. It seems that the only way to really understand it is to work in an OO environment with strongly distributed control for a while. Many people then say that they get a sudden "aha" when the style makes sense. At this point, their brains have been rewired, and they start thinking that decentralized control is actually easier.
Fowler – UML Distilled

1 comment:

bit-twiddler said...

Martin Fowler's opinion is just that, an opinion. Opinions are like anal orifices in that everyone has one.

I have been a professional software engineer for over three decades, and have spent the majority of my professional career working with OO languages and techniques (I started using OO techniques in 1984 with Ada). To this day, I write single entry/single exit point code that is non-convoluted.
Furthermore, guard clauses are reactive coding. Reactive coding is a sign of poor design. For example, operations such as range checking should be performed before value is assigned to a variable, not after it is passed as a parameter.

Private methods are not just for code that is used repeatedly within a class. They should be used to decompose public methods into concrete logical steps. Methods that use of multiple exit points are generally poorly factored. Any method that requires multiple exit points is a method that is performing several operations; therefore, it should be factored into smaller methods, each of which performs distinct one operation.