In the last article of TSM we discovered the universe of Clean Code written by Robert C. Martin. We had the opportunity to go deeper on the naming topic and see how easily small things like meaningful names or naming that revel intention can improve the quality and readability of the code itself.
Today we will dive deeper in Clean Code and we will talk about "Functions". This basic and simple mechanism used to write programs can impact not only how easily a program can be maintained and extended but also the mental health of developers. Don"t forget that long methods will make your eyes bleed.
Imagine a book where all the paragraphs are mixed, the font size is different for each of it and a part of them has 20 pages. How easily can you read a book like that? Code should be written in a way that gives people the opportunity to read it like a book, from top to bottom, where each different logic is grouped separately.
I remember one time when I had to extend an existing code written by somebody else. When I opened the solution I found only one class, with 2 or 3 methods that had in total around 4.000 lines of code. My estimations for that task were:
My PM from that period accepted this estimation and gave me green light to work on it. But in the end, I needed 4X more time to finish the task because the methods themselves were too long and I couldn"t do anything there without having nightmares during the nights.
So, what can we do to improve the quality of developers and our software from the perspective of functions/methods?
This is the first and the most important rule related to functions. You should keep them as short as possible. The explanation is pretty simple: a shorter function will do less (only one simple thing). Additionally to this, it will be more easily to understand and manage it.
The natural question that pops in our mind is "How short?".
100 lines?
50 lines?
10 lines?
5 lines?
Unfortunately we cannot have a magic number like 5 or 20, because it is pretty hard to generalize it. The length of a method depends on multiple factors like the code conventions. For example, how often you hit enter to add a new line (for every "{" or for every logic statement and so on).
In general, if you end up with a method longer than 10-15 lines of code you should take a look over it and see why it is so long. Because of code conventions or is it because there is too much logic there?
There is one thing about an IF, ELSE, WHERE, REPEAT and this kind of functionality. You don"t want to end up with an IF on 10 lines. It is pretty hard to read and understand. For this kind of cases you should extract the check into a separate function and call it from the IF statement. Appling this rule, you will end up with statements like IF or WHERE that need only one line of code.
Additionally to this, you will improve the readability and documentation of your own code. For developers it will be very easy to understand what the code does and what the check behind that IF or WHERE is supposed to do.
If you read a long function you realize that more than one thing is done there. For example in the same function you open the DB connection, you execute a query, you convert the result to another type and you handle special cases. Each of these things should be done in a separate place.
This is why a function should do ONLY one thing. If you need to do more than one thing, than you should split it into separate functions.
Even if the statement is so simple, it is pretty hard to do this. If you discover in a function different parts of code that are grouped or you can extract a part of it into a separate function with a meaningful name than the function is doing more than one thing.
This can be a mechanism that can tell us that the function is doing too many things. For example a function that processes an entity and also internally starts to split and transfer one of the entity fields has more than one level of abstraction.
It is pretty clear that you should extract the string processing into another function. In this way each function will have only one level of abstraction.
The story related to switch statements is pretty long and we will talk about it on another occasion. In our case we should extract the logic of each of CASE"s to separate functions. Even if we do this thing, it will not be okay because we are violating SRP (Single Responsibility Principle).
We should replace the switch statement with polymorphism. This is the perfect solution, but there are cases when such a solution would add to much extra complexity. When you need to use switch statement you should hide them as deep as possible, in Clean Code the recommendation is behind an abstract factor.
The name of a function should describe exactly what it does. Not less, nor more. For example, functions with names like "DO", "ACTION" are not very helpful, because we don"t know what their scope is.
A name like "TriggerDoorLock" gives us all the information that is needed to know what that function is doing.
Finding a good name is pretty hard and can consume a lot of energy. In addition to good names you should be consistent and try to use the same naming pattern when there are similarities.
How many arguments should a function have? The best value is 0, but it is not possible all the time. Each time when you add a new argument think about the role of it.
When you end up with more than 3 or 4 arguments something can be wrong. You should take a look over them and see if you cannot move the function to another location or add another abstraction level.
The OUT option to arguments is not all the time recommended and it may be a smell that something is wrong there. For example the TryXXX methods usually check if a conversion can be done. If it can be done with success, "TRUE" is retuned and the out parameter will contain the conversion result. This could be a smell that the method is doing too many things - converting and checking.
This is the case when your function is doing more than one thing, but without telling to the client. For example a "Read" method that reads the content of a file and after that deletes it without notify the user. In this case the user should be notified about this action or at least he should know about it from the moment when he does the call - "ReadAndDelete".
Because of these side effects we can end up with temporary coupling. For example when function "GoLeft" can be called only if "StartEngine" was called. You should think about a way to expose only the methods that are available at a specific time, without creating temporarily coupling. For example the "StartEngine" could return an object that has only the commands like "GoLeft", "GoRight" and so on.
A function should do only one thing. You should never have methods that execute a query and in the same time a command. These two actions need to be separated all the time without exception.
Returning an error code creates two additional things that need to be done by the developer/client. He needs to know the mapping of each error code and in the same time he has to check the returning error code.
For these cases, throwing an exception is better and will simplify the work of clients. Additional to this, the error handing will be 100% separated from your logic.
A code that contains this kind of blocks are pretty ugly and long. Because of this, all these blocks of code should be extracted into separated functions. The TRY block can be put into a function and the catch block can be put into another function.
All the code that is duplicated should be extracted to a separated function. You will not only reduce the number of lines of code, but you will also make your life easier when a change needs to be done. It is easier to change only one line of code than searching and changing all the locations where the code is duplicated.
As you can see, small things can make a real difference between a good and a bad function. You don"t need to do or know crazy stuff to be able to write "happy" functions. Taking into consideration this recommendation you will end up with better software that can be maintained after 10 years more easily and with less money.
by Ovidiu Mățan
by Dan Suciu
by Ovidiu Mățan
by Sorina Mone
by Ovidiu Mățan
