Writing readable and maintainable code

Writing readable and maintainable code

dev.to - Jan 23

"Quick notes on writing readable and maintainable code"

In the world of software development, writing code is not just about joining ends to make something work and hope for the best, it is necessary that each block written has a higher meaning - capable of predicting behaviors and working on them, that way. This way, it is necessary that the code be readable - and that it can be maintained quickly and efficiently.

A readable code is a requirement for any project. You cannot write loose things with low or zero sense or create unmaintainable levels of complexity: the key is balance. Doing what makes sense is necessary to guide the code and construct a coherent code.

🔹 What does "coherent" code mean?

Coherency, by definition: "logical and orderly and consistent relation of parts"(*1), so a coherent code is a logically compliant one: a code where you can assemble the logical steps to understand it's meaning, without tricks and implicit things.

Let's get an TypeScript example, a usual TypeScript code that follows steps to check user login status and permissions:

const user = someProvider.getLoggedUser();

if (user != null) {
    if (user.hasRole("admin")) {
        showMainPage();
        appendAdminPanel();
    }

    if (user.hasRole("member")) {
        showMainPage();
        appendCommonUserPanel();
        if (user.hasRole("subscriber") {
            appendSubscriberPanel();
        } else if (user.hasRole("veteran")) {
            appendVeteranBadge();
        }
    }
}
Enter fullscreen mode Exit fullscreen mode

What is the problem of that code? Actually, there are some obvious ones: nesting abuse, lack of error management, repeated similar blocks of code and if the user has various roles, there is a risk of conflicting screens being shown in the same page.

You can avoid unnecessary if-else nesting and big switch cases
using the "object literal" pattern. Note: this only applies to
a few, mainly dynamic, languages.

TypeScript Example:

const user = someProvider.getLoggedUser();
const actions = {
    any: showMainPage,
    admin: appendAdminPanel,
    member: appendCommonUserPanel,
    subscriber: appendSubscriberPanel,
    veteran: appendVeteranBadge
}

const roles = user.getRoles();

if (roles.length) {
    actions["any"]();
    roles.forEach(role => actions[role]);
}
Enter fullscreen mode Exit fullscreen mode

So, we have a common point where the code starts getting complex and loses its maintainability: entropy.

🔹 What is "entropy"?

In short words, entropy is a measurement made to know how chaotic is a system - not only in the programming world but in the entire known universe. Now, getting back to the code, a good example of higher than normal entropy (and - in my opinion - a bad language design choice) is the C# keyword out. Let's get the C# snippet below as an example:

if (int.TryParse("9", out int parsed)) 
{
    Console.WriteLine(parsed);
}
Enter fullscreen mode Exit fullscreen mode

You can notice that the variable parsed is being implicitly declared: this is a mess. Just looking at the code, you don't know that a variable is being declared, the variable scope and how to deal with the side effects of this variable existing. The method itself also returns a boolean, branching the expression objective and doing more than one direct operation.

Unfortunately, this is a language design decision, so making workarounds - in the most of the time - will not be better than that implementation.

🔹 Dealing with entropy

While writing a method, it's a good thing to think about: what should this method do? What are the consequences of using this method inside or outside an expression? Let's get a C# example:

public byte[] GenerateReportAndConvertToPdf(
    string clientId, 
    string operationId) 
{
    var operation = _operationService.GetById(operationId);
    var client = operation.GetClientById(clientId);

    var html = client.GetHtmlReport();

    var pdf = SomeProvider.ConvertToPdf(
        html, 
        SomePageSize.A4);

    using var pdfStream = new MemoryStream();
    pdf.SaveTo(pdfStream);

    return pdfStream.ToArray();
}
Enter fullscreen mode Exit fullscreen mode

What is wrong with that code? Here, we can see coupled responsibilities into a single method, this code can not be correctly tested, debugged or even properly maintained because the operations have different objectives but are being implemented as one single action.

A better implementation of that pipeline could be separated in more methods:

//CSharp
public string GenerateReport(string clientId, string operationId)
{
    var operation = _operationService.GetById(operationId);
    var client = operation.GetClientById(clientId);

    return client.GetHtmlReport();
}

public byte[] GeneratePdf(string html)
{
    var pdf = SomeProvider.ConvertToPdf(html, SomePageSize.A4);

    using var pdfStream = new MemoryStream();
    pdf.SaveTo(pdfStream);

    return pdfStream.ToArray();
}
Enter fullscreen mode Exit fullscreen mode

🔹 Nesting and indirect operations

Foremost: what are indirect operations? They are blocks of code with uncertain objectives that can easily be made without thinking about the complexity of your code. Common examples are abusing of nested blocks - like creating a callback hell or making various loops of loops.

A common example of callback hell in JavaScript can be seen below:

fetch("someapi/user/someId")
    .then(data => data.json())
    .then(user => {
        fetch(`otherapi/books/${user.books[0].id}`)
            .then(data => data.json())
            .then(book => console.log(book.name)
    })
Enter fullscreen mode Exit fullscreen mode

The problem is obvious: the entropy tends to be bigger for each step needed to be made in that operation. If we wanted to access a third API and get extra data, the operation would be even bigger and unreadable.

In another example, we can recognize the same problem when needing to make complex list iterations, in basic cases the first idea is using for-loops, but it can produce a terrible result, like in this Rust example:

let mut school_student_ages: Vec<i32> = vec![];

for city in cities {
    for district in city.districts {
        for school in district.schools {
            for class in school.classes {
                for student in class.students {
                    school_student_ages.push(*student.age);
                }
            }
        }
    }
}
Enter fullscreen mode Exit fullscreen mode

This problem can be solved using various ways, but the fastest one is using a functional API:

let school_student_ages: Vec<i32> = cities
    .into_iter()
    .map(|city| city.districts)
    .map(|district| district.schools)
    .map(|school| school.classes)
    .map(|class| class.students)
    .map(|student| student.age)
    .collect();
Enter fullscreen mode Exit fullscreen mode

💬 Note

Some languages doesnt have functional APIs and this example can not be directly reproduced in them.

🔹 Dealing with duplicated code

Duplicated code is one of the most frequent problems that occurs while writing code and is more frequent at large codebases - usually manipulated by many developers. Let's suppose that you have to make a new endpoint that creates a new custom to-do item and saves it to a list - in this context, we are using C# (ASP.NET Core) with a Controller - Service - Repository architecture.

💬 Note

Controller - Service - Repository is a design pattern where controllers are classes that contains the endpoint methods, services are classes that contains the business logic to be applied between the transactions and repositories are the final layer that handles the data connection.

To create a new endpoint, first you have to create the repository method like that:

public async Task CreateCustomTodo(Todo todo)
{
    await _context.Todos.AddAsync(todo);
    await _context.SaveChangesAsync();
}
Enter fullscreen mode Exit fullscreen mode

After, you need to create the service method:

public async Task CreateCustomTodo(Todo todo, TodoSpec todoSpec)
{
    //... some logic with todo and todoSpec ...
    await _repository.CreateCustomTodo(todo);
}
Enter fullscreen mode Exit fullscreen mode

Then, the controller method will look like that:

[HttpPost("/todo/custom")]
public async Task<IActionResult> CustomTodo(
[FromBody] createCustomTodo)
{
    var data = SomeDestructuringMethod(createCustomTodo); 
    await _service.CreateCustomTodo(data.todo, data.todoSpec)
    return Ok();
}
Enter fullscreen mode Exit fullscreen mode

What is wrong with that code? It could be generic. The service and controller layer are ok, but the repository is doing a specific action - other developers will make more and more methods that are doing the same thing without even knowing about your method. This is a simple thing - but in my career I've seen this happening so many times that I couldn't ignore the fact.

The only fix that this code needs is changing the repository method CreateCustomTodo name to CreateTodo since no business rule or additional logic is being applied to it.

References and credits

🔹 (*1) Coherency definition by vocabulary.com

MORE ARTICLES