How to write clean code - Part I

Intro
Clean code principles... every professional software engineer should know about them and most importantly, follow them. Software engineering as a profession mentions the first time in 1963 while developing guidance and navigation systems for the Appolo missions, so it exists for a long time, almost 60 years. In that period a lot of problems were encountered and solved with various design patterns, conventions, and principles. So this blog serves as a review of those rules which can help our team and ourselves to be better software engineers. Everything which is mentioned here is inspired by the book "Clean Code, A Handbook of Agile Software Craftsmanship" written by famous author Robert C. Martin aka Uncle Bob. The main thought through all coding sessions should be: "clean code should always read like well-written prose." by Grady Booch.
Naming
TLDR
- Use meaningful and intention revealing names in your code
- Short scope names should be short
- Large scope names can be longer but must be more descriptive
Can you remember the days when you first started coding? Those "Hello world" and "Your name is" programs, well let's go a bit further and try to remember the first algorithm you wrote, an array sorting, or maybe find in the collection piece. Probably back then we weren't thinking much about our variable names and we were naming them like:
int a;
int xyz;
String s;
int[] a;
Sure, for short programs like those were, it could pass, but imagine using names as those in a big, complex codebase. You'll surely agree that it would be really hard to read and understand what is happening. There are cases where we can use short names like that, but we'll get to that quickly.
Here is an example code from the early days:
public static void bubbleSort(int[] a) {
boolean s = false;
int t;
while(!s) {
s = true;
for (int i = 0; i < a.length - 1; i++) {
if (a[i] > a[i+1]) {
t = a[i];
a[i] = a[i+1];
a[i+1] = t;
s = false;
}
}
}
}
This is a standard bubble sort algorithm that consumes an array and doesn't return anything. Take a look at the name of a variable, is it self-explanatory, do you know from the name what it represents?
Now look at this code:
public static void bubbleSort(int[] array) {
boolean isSorted = false;
int biggerValue;
while(!isSorted) {
isSorted = true;
for (int i = 0; i < array.length - 1; i++) {
if (array[i] > array[i+1]) {
biggerValue = array[i];
array[i] = array[i+1];
array[i+1] = biggerValue;
isSorted = false;
}
}
}
}
bubbleSort()
is a verb. Maybe you noticed the index variable in the for loop is just the letter i
, but that is fine because we are all used to it, and we know what i
stands for. Another reason why is so short name is that the scope of it is very short, only inside the loop. That brings me to another rule which says that the length of a variable name should depend on its scope. If the variable is more of local scope, its name should be short, but if it has a larger scope, a more descriptive name would be a better choice.Functions
TLDR
- Always separate functions by their level of abstraction
- The number of arguments shouldn't be more than 4
- It would be better to create functions for true and false cases instead of passing a bool as an argument
- If possible, functions should be around 4 lines long.
- The function should do ONE thing and ONE thing only
- The function should not have any side effects
Again, let's remember the first days of our programming lessons. In those lessons, we were told to use functions when there is repeating code, to make it reusable. Let's google the definition and see the result, I found the following: "A function is a block of organized, reusable code that is used to perform a single, related action." So we will use that code a lot and that is why it is important to make it clean and readable. How to achieve that? Firstly, we should keep our functions short, according to Uncle Bob, 4 lines top. While that is ideal, it isn't written in the stone or compiler, so it can be a bit longer, but keep it as short as possible. What Uncle Bob wants to say with 4 lines tops functions is to extract the code from the functions to more short and simpler functions, but why? Look at the example:
public RedirectView authenticateUserAndRedirectToMainPage(String username, String password) {
Optional<User> user = Optional.empty();
DataSource dataSource = DataSourceSingleton.getInstace();
try (Connection connection = dataSource.getConnection();
CallableStatement stmt = connection.prepareCall("authenticateUser")) {
stmt.setString(1, email);
stmt.setString(2, password);
try (ResultSet rs = stmt.executeQuery()) {
if (rs.next()) {
user = Optional.of(new User(
rs.getInt("ID"),
rs.getString("EMAIL"),
rs.getString("PASSWORD"),
rs.getBoolean("IS_ADMIN"))
);
}
}
} catch (SQLException ex) {
Logger.getLogger(DBUserRepository.class.getName()).log(Level.SEVERE, null, ex);
}
if (user.isPresent()) {
RedirectView redirectView = new RedirectView();
redirectView .setUrl("http://www.stackoverflow.com");
return redirectView;
} else {
throw new RuntimeException("User not found...");
}
}
"authenticateUser"
and get the result if the user exists. We can agree that this is a low level of abstraction. One more thing to worry about is the try-catch block that surrounds the database access code. Let's solve the problem by extracting and refactoring the code. Before we start, just one note that we will focus on refactoring the authenticateUserAndRedirectToMainPage
function to keep it simple. Others will be ignored.The first step is to extract the try-catch block to function.
private Optional<User> authenticateUser(String username, String password) {
DataSource dataSource = DataSourceSingleton.getInstace();
try (Connection connection = dataSource.getConnection();
CallableStatement stmt = connection.prepareCall("authenticateUser")) {
stmt.setString(1, email);
stmt.setString(2, password);
try (ResultSet rs = stmt.executeQuery()) {
if (rs.next()) {
return Optional.of(new User(
rs.getInt("ID"),
rs.getString("EMAIL"),
rs.getString("PASSWORD"),
rs.getBoolean("IS_ADMIN"))
);
}
}
} catch (SQLException ex) {
Logger.getLogger(DBUserRepository.class.getName()).log(Level.SEVERE, null, ex);
}
return Optional.empty();
}
public RedirectView authenticateUserAndRedirectToMainPage(String username, String password) {
Optional<User> user = authenticateUser(username, password);
if (user.isPresent()) {
RedirectView redirectView = new RedirectView();
redirectView.setUrl("http://www.stackoverflow.com");
return redirectView;
} else {
throw new RuntimeException("User not found...");
}
}
private RedirectView redirectToMainPage() {
RedirectView redirectView = new RedirectView();
redirectView.setUrl("http://www.stackoverflow.com");
return redirectView;
}
public RedirectView authenticateUserAndRedirectToMainPage(String username, String password) {
Optional<User> user = authenticateUser(username, password);
if (user.isPresent()) {
return redirectToMainPage();
} else {
throw new RuntimeException("User not found...");
}
}
loginUser
. If it is a bit confusing how is this good, because our function is doing two actions again, authentication and redirecting, well we have to merge them to make a feature. It is a higher level of abstraction, therefore more abstract naming and function body. The final result should look like this:public RedirectView loginUser(String username, String password) {
return authenticateUser(username, password)
.map(user -> redirectToMainPage())
.orElseThrow(() -> new RuntimeException("User not found..."));
}
This section was a bit longer, but we covered a lot of important rules and what should be avoided while writing our code.
Comments
TLDR
- Avoid writing comments
- Make a code explain itself, then there is no need for comments
- A lot of comments is a sign of bad code
- Good comments: legal, todo, javadocs...
- Commented-out code is a sin
The first and most important rule about comments is not to write them. Avoid them as much as possible. Think of writing a comment in the code like littering the street. Why? Let's again go back to school days, probably we would think comments are helpful because they describe what the code does and maybe keep the track of all changes made through the development. And partially, yes, we would find that helpful, to read a comment as a description of some complex algorithm, but when we write code as in the previous section, like well-written prose, comments are unnecessary. Code should be self-explanatory, do you agree? When we write functions that are concise, short, and well named, comments really are like litter on the street. So if you see lots of comments in the code, that is a badly written code. Even in the development process, avoid the comments, avoid them to log the changes, because most of the modern IDEs today have that integrated. IntelliJ for example has Git to show history, when it was edited, by whom, and much more. Also, avoid commenting out the code you don't use anymore because you think you might need it at some point, but probably you won't. Right, we talked about bad comments, but actually, there are some use cases where comments are a good practice. The first example is Javadoc comments. For instance, when we use an external library sometimes we need more details, for example, about the function we call. In IntelliJ by pressing Ctrl + left mouse click, it opens the code file where we can find Javadoc written by the author of a class. This is an example of how Javadocs look:
/**
* Reads single {@link CodeBook} from database specified by {@link StaticValue}
*
* @param staticValue value to load from database
* @param <I> {@link javax.persistence.Id} type
* @param <T> {@link javax.persistence.Entity} type
* @return single {@link CodeBook}
*/
<I extends Number, T extends CodeBook<I>> T load(StaticValue<I, T> staticValue);
Formatting
TLDR
- IDEs now do it for ourselves
- Its role is to make code easy to understand
- Each company or team have its own rules
This is a short topic because formatting depends on the company in which you work and your personal preferences, so there are no strict rules. Since there are no strict rules, every company has its formatting, its code style. Let's see two examples of different formatting.
if (clause) {
code_to_execute
} else {
code_to_execute
}
if (clause) {
code_to_execute
}
else {
code_to_execute
}
Conclusion
All right, a lot of topics were covered above and these take practice learning and making them a habit. Try the approach in which you write the solution for the problem first and clean the code afterward. That way, you'll have the image in your head of what the solution should look like after you write it down. Remember to name your variables, constants, methods, and classes properly, make sure that your functions do ONE thing and ONE thing only and classes should have ONE responsibility and ONE responsibility only. As your code grows, so should you as a professional, therefore clean your code. We have a lot more to cover so stay tuned for the next part of this blog.