Writing Good and Readable Functions: Clean Code!
In this blog post series I will go through the most important topics concerning writing good code, that every programmer should be aware of. Most of the tips will be based off of Robert C. Martin's Clean Code and my own experiences. I will use code I wrote in my first year in college as examples. In this chapter we will focus on how to write good readable functions, that do what they say.
Small functions that do one thing
If you are a serious programmer, I hope it does not surprise you that functions should be as small as possible. The problem with large functions is that if they take too long to read, the reader will forget the context or what the function was supposed to originally do, by the time they have read through it. The function might even do more than what it promises.
If there is one thing to take out of Clean Code, it is that a function should do one thing. I think this is the most important part of the book, since understanding a function that truly does one thing is very easy. Not only does the function usually become small enough to read it without scrolling, but when using it in conjunction with other functions that do a single thing, main methods will also become easier to read. If the functions are named properly after what they do, we would not need to read the function, but reading the main function would suffice in understanding the program. Take a look at the following code:
/** Stores a file path from user input.
* @return The file path.
*/
public static String userInput(){
String input = "";
/* will continue to ask for path name until
* x != 0, namely, until file path is acceptable.
*/
while (true){
// take input
Scanner in = new Scanner(System.in);
System.out.println("Type full path name of sudoku file");
input = in.nextLine();
File f = new File(input);
/* if file path contains a file
* check if it's a txt file.
*/
if(f.isFile())
{
/* if extension is txt
* then break out of while loop
* return file path
*/
if (FileExtension(input).equals("txt"))
{
in.close();
break;
}
/* if extension is not txt
* ask for new input
*/
else{
System.out.println("Input file must be a .txt file");
System.out.println("n**RESTART**n");
}
}
/* if input is not a valid file path
* ask for new input
*/
else
{
in.close();
System.out.println("That is not a path to a file, try again.");
System.out.println("n**RESTART**n");
}
} // End of while loop
// Return input
return input;
} // End of userInput method
Briefly skim-reading it, you should realize that the code is too long and complex for what it does. Reading the name, will not tell you the purpose of the code. Something with user input, but what? Here's everything wrong with this code:
- Name does not say what the function does.
- Comments are unnecessary and only make the code harder to read.
- The code does more than one thing: Taking input and validating it.
- Nested if-statements instead of using "&&".
- The code initializes a new scanner and File object every time it loops instead of using the already opened scanner.
Let us rewrite the function so it does only one thing:
public static String getPathFromUserInput(Scanner in){
System.out.println("Type full path name of sudoku file");
String input = in.nextLine();
File path = new File(input);
if(isPathTextFile(path)) {
return input;
}
else {
return getPathFromUserInput(in);
}
}
By getting rid of unnecessary comments, and encapsulating code that does something else than specified by the function name into another function, we have shrunk the function from 55 lines to only 13 lines. By naming the function by exactly what it is doing, keeping the code short and simple, no additional commenting is needed. The code speaks for itself, success!
You could even save another two lines by excluding the else statement like this:
public static String getPathFromUserInput(Scanner in){
System.out.println("Type full path name of sudoku file");
String input = in.nextLine();
File path = new File(input);
if(isPathTextFile(path)) {
return input;
}
return getPathFromUserInput(in);
}
However some people do not like this coding style, since it can get confusing in some contexts.
Using descriptive names and avoiding comments
You might say getPathFromUserInput is a long name for a function. I agree and I would avoid it as much as necessary, as it can become tiresome to read a long function name. Especially if you have a class with many long function names. But I'd take it every time over the 23 lines of comments I had before. Aim for a function name that explains to a reader everything a function does. The problem with comments is that they rot and lie. This happens when code gets revised and the programmer forgets to check what the comments are saying, and if it is on par with what the function does now. In a big code base written by many programmers, this problem can grow ugly. Naming a function correctly to avoid comments is therefore crucial, even if the name becomes long.
Did you read the first code-listing in this blog-post and think wtf is x ? Yes? That is why comments suck. The comment says something about a variable named x that doesn't exist. It probably did exist and had a key feature, but after revising the code, I must have deleted it. Yet the comments talk about x. This is exactly why you should avoid comments if you can.
When deciding on a function name, consider if it is possible to use a noun/verb combination for the function and its arguments so they can be read as a sentence. For example load(page) is one such combination. It is easy to read what the function does and what the argument is used for.
The number of arguments should be kept to a minimum. If possible, try not to pass any arguments at all. The more arguments your function has, the harder it gets to come up with a descriptive function name that makes sense of what the arguments are used for. Testing also becomes harder the more arguments a function has, since you have to try all kinds of argument combinations. If there is no argument, this task becomes trivial.