Intro

Some time ago I read Clean Code, a very good book by Uncle Bob on how to write clean code, or should I say: how to polish written code until it becomes cleaner.

My first read through is something he calls at the beginning of the book a “feel good read” as I didn’t work hard and sweat, so I haven’t yet the full benefit of the book.

But I think with this first read I’ve gotten the general Idea and I have some quibbles about some parts of the book. I feel I should address them.

This is usual for me when I read his writings, some things I agree, a few I disagree and a couple I strongly disagree and once I very strongly disagree and then a I strongly agreed.

The one In this book I strongly disagree and made me start to write this post is about the exceptions, and then I let this post sitting for some time.

But I came across some people following the advice and I felt I should come back and publish this.

As he said in the beginning of Chapter 16, this is not in malice and I do not think myself better than him, my code is as flawed as the next guy as you can see from my contributions to the [Layout Parser] (http://sourceforge.net/projects/layoutparser/).

I have extracted from the book a lot of good advice which I shall incorporate in my own coding practices, there are some that are contrary to my believes and made me, at least, question the truth in them.

Yet there is always benefit in reviews and feedbacks.

Do not mislead

It is very well stated in the book that one should not mislead his reader and the code should do the least surprising thing and that functions should do one thing.

I happen to agree with that and I find the function renderPageWithSetupAndTeardowns in Chapter 3 given as example and refactored until clean to be still offending those principles.

If I were reading a code that makes a call to that function I would just assume that it decorates the page with setup and teardowns, on a debugging session searching for why the page is not being rendered with them it would be the last place I would look for the test if(isTestPage), I would expect this decision would already be made in order to call a function called renderPageWithSetupAndTeardowns.

So it surprises me by doing two things which is rendering the page and deciding if the render will be with or without setups and teardowns.

Javadocs

There are some criticism about javadocs should be written:

• Should not express what function and parameter’s names already convey.
• Should not contain mark-up.

But javadoc is not code and the audience for javadocs is not programmers maintaining the code, javadocs are for the API clients and thus it should not rely on the code.

With modern editors you can simply collapse the javadoc comments, if they are too bulky for your taste, and just ignore them.

So the content of javadoc should be as complete and useful as it could be without being constrained by rules intended by code readability. They are two different concerns.

Checked exceptions

Here is where I think the book is deadly wrong and it is even doing a disservice to the craft, he even declares the debate over, personally I think no one is ever capable of declaring a debate over.

Anyone that is regular to any forum can attest to that…

Let’s look at his arguments:

The price of checked exceptions is an Open/ Closed Principle violation. If you throw a checked exception from a method in your code and he catch is three levels above, you must declare that exception in the signature of each method between you and the catch. This means that a change at a low level of the software can force signature changes on many higher levels.

Note that if a given function throws an exception, and this exception will be meaningfully handled three levels above the open/closed principle is already compromised.

Regardless the fact it is declared or not, there is a distant class dealing with it. If it come to pass that the exception is changed the handler class will have to be changed too.

On top of that if an exception was allowed to merrily float up, that means it went up likely three abstraction layers and we have a high abstraction layer dealing with low abstraction guts.

Which brings us to the next argument against indiscriminate use of unchecked exceptions: they favour leakage of abstraction.

Through checked exceptions, java forces you deal with the possible problems that may arrive, letting the abstraction bubble to burst must be an active decision.

On the other hand, with unchecked exceptions you may not even be aware the abstraction is leaking until it happens and that may mean production time.

Which brings us to yet another argument against unchecked exceptions: they violate the principle of least surprise.

What a piece of code does is not merely a question of what the happy path does, the way the code fail is equally relevant. When I come across a signature which says:

File open(String path) throws FileNotFoundException;

It is telling straight away everything that can happen. I can mock the object and make the mock throw the exception and then I can write code to deal with it, and if I decide to let the exception go up I can wrap it up in a meaningful exception according to my abstraction, say: UnableToLocateCustomerException.

On the other hand if it is all unchecked and the client of my code has the interface:

Location locateCustomer(Customer customer);

He will have quite a surprise to see a FileNotFoundException to blow his nose, it is completely unexpected, as the Spanish inquisition, he does not care and should not care if my code uses files, data bases or smoke signal to store its data.

Declaring the throws clause in every method in the away the exception travels is not the dirty part of the code.

Letting it go up the abstraction and handle it where the exception is less meaningful and there is less chance of doing something useful with it, this is the dirt.

Making them unchecked merely hides the dirt under the rug and makes it harder to test and allow the dirt to grow mould until you find about because you are sick.