Telerik blogs

Clean code is a fundamental concept in software engineering that focuses on writing readable, understandable and maintainable code. This blog post will cover tips on how to write clean code in ASP.NET Core applications.

The term “clean code” is very well-known in application development. Both beginners and experts must understand this concept to create robust, scalable and easy-to-maintain systems.

In this blog post, we will understand the concept of clean code, the principles behind it and how we can apply it to our code through several practical examples.

The post is separated into five main topics, which were selected considering the applicability and importance of applications built in ASP.NET Core.

What Is Clean Code?

Clean code is a software engineering term that was popularized by the book of the same name by Robert Cecil Martin (Uncle Bob). Far beyond code, the term “clean code” refers to documents, concepts, rules and procedures that are intuitively understandable by developers.

Following the concept of clean code, anything that can be understood correctly with little effort and in a short time is considered intuitively understandable is considered “clean.” Writing clean code encompasses the clarity and expressiveness of all artifacts produced in the software development process, including documents, diagrams and user interfaces.

The essence of clean code lies in the ability to effectively communicate the intent behind the software, making it understandable not only to whoever initially wrote it but also to anyone who interacts with it later. This is crucial in collaborative environments and projects that require maintenance and evolution over time.

Additionally, the clean code concept promotes a mindset of responsibility and respect for teammates and end users of the software.

By writing code and creating artifacts that are clear, concise and easy to understand, developers demonstrate a commitment to quality, transparency and collaboration. It is important to note that the pursuit of “cleanliness” in code and development processes should not be seen as an end in itself, but rather as a means to achieving broader goals of quality, efficiency and customer satisfaction. It’s a guiding principle that can help development teams work more effectively and create software of greater value and impact.

Below we will check out five topics that represent clean code, implementing practical examples in ASP.NET Core.

You can access the source code of the examples here: Practicing Clean Code - Source Code.

1. Writing Good Names

Write Meaningful Names

Writing meaningful names is fundamental in software development, especially in ASP.NET Core projects, where creating different types of relationships between objects is possible.

Writing code is like writing a book, where the author tries to express an idea aiming for a clear understanding by the reader, so it is important to write names that represent what the code does. See the following example.

Imagine that you need to create a method to send a notification to the managers of all employees who will be on vacation in the current month. However, there is an important detail: only employees who have already completed one year of work at the company can be on this list.

If we were to create a method to process them without using the concept of clean code, we could do it like this:

public void SendNotification()
{
  List<Notification> eligibleEmployees = GetElegibleEmplyees();
    _employeeRepository.Notifications.AddRange(eligibleEmployees);
    _employeeRepository.SaveChanges();
}

Note that this method only informs the reader that it sends a notification, but its name gives few details about this sending, such as what type of notification it is.

However, when using clean code concepts we can rewrite it as follows:

public void SendVacationNotificationToEligibleEmployees()
{
  List<Notification> eligibleEmployees = GetElegibleEmplyees();
    _employeeRepository.Notifications.AddRange(eligibleEmployees);
    _employeeRepository.SaveChanges();
}

Now the method’s name is much longer than in the first case, but it is no longer generic—it provides important details about the method, and now we know that it sends vacation notifications to eligible employees.

It may seem like a small detail, but when we are reading code that does specific things, details like this make a big difference. Through meaningful names, developers can draw a more complete line of thought when analyzing code, being able to find possible bugs more easily.

The previous example is used in a method, but we can extend the same principle to variables, classes, functions, interfaces, etc.

Let’s analyze the following example:

public decimal CalculateAbsenceDiscount(decimal a, int p)
  {
    if (a != 0) 
    {
      int days = DateTime.DaysInMonth(DateTime.Today.Month, DateTime.Today.Year);
      return p / days * p;
    }
  return 0;
}

Note the method above with the good name that clarifies its intention (calculating the absence discount). However, when we look at the name of the method’s input parameters, we soon realize that there is something wrong. What do the parameters “a” and “p” mean? Would it be the initials of “absences” and “paycheck”? In this case, we can say yes, but if a reader looks at the code for the first time, they might be confused.

Also note that a variable called “days” is being created, which doesn’t say much, as it is a generic name and nothing specific. In addition, the output of the DaysInMonth method is an integer of the total number of days in a month depending on of the year and month informed when signing the method. We can conclude that this is a terrible bit of code, as it is both difficult to read and confusing.

Now, notice the code below just changing the name of the variables.

public decimal CalculateAbsenceDiscount(decimal absences, int paycheck)
{
  if (absences != 0)
    {
      int totalDaysOfMonth = DateTime.DaysInMonth(DateTime.Today.Month, DateTime.Today.Year);
      return paycheck / totalDaysOfMonth * absences;
    }
  return 0;
}

See how much clearer it is to read this code? All names follow their real function, where the value of the paycheck divided by the total number of days in the month multiplied by the number of absences results in the value of the discount for absences.

Write Searchable Names

When we talk about meaningful names, we must remember that in addition to being clear, these names must be searchable. In a program, there can be hundreds, thousands, even millions of names. Imagine searching for a specific name within a tangle of code.

Although current Integrated Development Environments (IDEs) facilitate the research process, if we don’t write good names, this work can take much longer than expected. So avoid abbreviations like the example below:

int[] comp_Cod_Elegibles = { 398431, 339292, 394939, 919281, 929191, 382811 };

Here we have an abbreviated name, followed by underlines. This is a bad practice because abbreviated names, in addition to confusing the reader, make the process of searching for names difficult. See the image below doing a search in Visual Studio Code for the name “comp”:

Searching names bad example

When searching for the abbreviated name “comp,” the search returns four results. This is because files with the name “component” were also brought up. If there were other files with this abbreviation, they would also appear in the search result, making the process more difficult.

If we change the example code to:

int[] companyCodesEligibleForInvoicing = { 398431, 339292, 394939, 919281, 929191, 382811 };

and search for “company,” the result will be exactly what we are looking for:

Searching names good example

Respect Conventions

“Naming convention” refers to a set of rules and guidelines that developers follow when naming classes, methods, variables and other elements in their programs.

These conventions are important to ensure uniformity between different projects, they are not an arbitrary rule, but serve as a good guide to standardize different programming styles and can be different depending on the language.

In ASP.NET Core we use C# as a language where we have several conventions for names, as you can see in this article on the Microsoft website: Coding style.

Many developers, due to lack of knowledge or for other reasons, do not follow naming conventions, writing codes like the example below:

string _completeName = $"{employee.Name} {employee.Lastname}";

Note that in the code above the variable _completeName is created, which has an underscore (_) at the beginning of the name. Although the C# compiler allows it, this is a bad practice as it goes against the convention that says: “Private instance fields start with an underscore (_).” Note that the convention talks about instances; in the example above, a variable of type string is being created—that is, there is no need to use the underscore.

A classic and correct example of using underscore would be the following:

private readonly EmployeeRepository _employeeRepository;

Here we are using the underscore when creating an instance of the EmployeeRepository class. The convention uses this directive because otherwise we would have to use the this operator in the class constructor to differentiate the instances—something already outside good C# practices.

2. Writing Good Functions

Functions play a fundamental role in programming and writing clean code. Inside functions are where much of the business logic resides in a software system. Business rules represent the policies, procedures and operations that define how an organization conducts its business. Therefore, the functions that implement these rules must be well-designed, clear and efficient.

Below we will check some points that should be considered when creating good functions.

Create Small Functions

The phrase above may seem obvious, but it is essential and we often forget it.

When we’re extracting an idea from our heads and putting it into code, it’s very easy to create a giant function that has everything it needs to do its job. However, we must consider that, although it may work exactly the way we need it, if it is large, it means that it is failing in several aspects such as transparency, readability, flexibility and separation of interests, among others.

Let’s analyze the code below:

public void ProcessNationalEmployeeVerification()
{
  var nationalEmployees = from employee in _employeeRepository.Employees
    join company in _employeeRepository.Companies on employee.CompanyId equals company.Id
    where company.National
    select new
    {
      EmployeeId = employee.Id,
      EmployeeName = employee.Name,
      EmployeeLastname = employee.Lastname,
      EmployeePaycheck = employee.Paycheck,
      CompanyName = company.Name
    };

  var nonNationalEmployees = from employee in _employeeRepository.Employees
    join company in _employeeRepository.Companies on employee.CompanyId equals company.Id
    where company.National == false
    select new
    {
      EmployeeId = employee.Id,
      EmployeeName = employee.Name,
      EmployeeLastname = employee.Lastname,
      EmployeePaycheck = employee.Paycheck,
      CompanyName = company.Name
    };

  var rates = new List<Rate>();
  var notifications = new List<Notification>();

  foreach (var nationalEmployee in nationalEmployees)
  {
    decimal totalRateAmount = nationalEmployee.EmployeePaycheck * (decimal)TaxRates.NationalEmployee / 100;

    rates.Add(new Rate(Guid.NewGuid(), totalRateAmount, nationalEmployee.EmployeeId, false));

    notifications.Add(new Notification()
    {
      Id = Guid.NewGuid(),
      EmployeeName = nationalEmployee.EmployeeName,
      NotificationType = NotificationType.RatesRegistration
    });
  }

  foreach (var nonNationalEmployee in nonNationalEmployees)
  {
    decimal totalRateAmount = nonNationalEmployee.EmployeePaycheck * (decimal)TaxRates.NonNationalEmployee / 100;

    rates.Add(new Rate(Guid.NewGuid(), totalRateAmount, nonNationalEmployee.EmployeeId, false));

    notifications.Add(new Notification()
    {
      Id = Guid.NewGuid(),
      EmployeeName = nonNationalEmployee.EmployeeName,
      NotificationType = NotificationType.RatesRegistration
    });
  }

   _employeeRepository.Notifications.AddRange(notifications);
   _employeeRepository.SaveChanges();
}

This code is apparently well written and not that difficult to interpret. First, it creates two lists of national and non-national employees, then calculates the discount rate for each, then creates a rate list and a notification list, and finally saves changes to the database.

Although well written, this code deviates from the first principle of clean code for functions: Write small functions! A function with 60 lines is certainly not a small function, and it is easy to identify places where we can separate this function into smaller functions.

Another problem with this function is that it deviates from the first SOLID principle—single responsibility—which says that each class should have only one responsibility. This function does several things, so we need to find a way around it. Let’s refactor this function and break it into smaller functions.

public void ProcessEmployee()
{
  ProcessEmployeeRates();

  ProcessNotification();

  _employeeRepository.SaveChanges();
}

public void ProcessEmployeeRates()
{
  var nationalEmployees = GetEmployeesByNationality(true).ToList();
  var nonNationalEmployees = GetEmployeesByNationality(false).ToList();

  CalculateEmployeesRates(nationalEmployees, true);
  CalculateEmployeesRates(nonNationalEmployees, false);
}

private IQueryable<Employee> GetEmployeesByNationality(bool isNational)
{
  return from employee in _employeeRepository.Employees
    join company in _employeeRepository.Companies on employee.CompanyId equals company.Id
    where company.National == isNational
    select employee;
}

private void CalculateEmployeesRates(IEnumerable<Employee> employees, bool isNational)
{
  decimal taxRate = isNational ? (decimal)TaxRates.NationalEmployee : (decimal)TaxRates.NonNationalEmployee;
  var rates = new List<Rate>();

  foreach (var employee in employees)
  {
    decimal totalRateAmount = CalculateRateAmount(taxRate, employee);

    rates.Add(new Rate(Guid.NewGuid(), totalRateAmount, employee.Id, false));
  }
}

private decimal CalculateRateAmount(decimal taxRate, Employee employee)
{
    return employee.Paycheck * taxRate / 100;
}

private void ProcessNotification()
{
  var notifications = _employeeRepository.Employees
    .Select(employee => new Notification()
    {
      Id = Guid.NewGuid(),
      EmployeeName = employee.Name,
      NotificationType = NotificationType.RatesRegistration
    })
    .ToList();

  _employeeRepository.Notifications.AddRange(notifications);
}

The code above is now organized in a modular way, where each function has a clear objective and only does what is responsible for it.

This way, maintaining the code becomes much easier, because if it is necessary to add any new logic or business rule, it is easy to identify which part of the code should be changed.

To facilitate the refactoring work, you can take advantage of IDE resources. Visual Studio has refactoring functions where you select a piece of code and can choose the “Extract method” or “Extract local function” options.

Create Functions with a Maximum of 3 Parameters

Creating functions with a limited number of parameters is a recommended practice in programming for several reasons such as code readability, ease of use and reduced complexity.

Ideally, a function should receive only one parameter, but if this is not possible, pass a maximum of three. More than that, it will be necessary to follow some strategies:

  • Grouping related parameters: If a function needs more than three parameters, it can be useful to group them in a data structure, such as an object, and pass only that object as an argument.
  • Functionality division: If a function is doing many different things, it may be a sign that it needs to be divided into smaller, more specific functions, each with fewer parameters.

Note the method below:

public void ProcessAbsenteeismNotification(List<Employee> employeesWithAbsenteeism, decimal taxRate, string notificationText, bool isNational, List<string> receivers)
{
  //...rest of the code
}

It has five input parameters—something that hinders the reading and maintenance of this code. To resolve this, we can create a parameter object and instead of passing each of the separate parameters, we pass only the object:

public void ProcessAbsenteeismNotification(NotificationData data)
{
  List<Employee> employeesWithAbsenteeism = data.Employees;
  decimal taxRate = data.TaxRate;
  string notificationText = data.NotificationText;
  bool isNational = data.IsNational;
  List<string> receivers = data.Receivers;

  //...rest of the code
}

Here we have the same parameters as before, but now they are all inside the NotificationData class and only this object is passed to the method, making it easier to read and modify.

Avoid Creating Side Effects

Side effects in functions refer to the changes or modifications a function can make to external variables or objects other than those it was designed to modify.

In simple terms, methods that have side effects lie! They say they do one thing but they do another.

These side effects can impact other parts of the system, often in unexpected or unwanted ways, resulting in bugs or the best-case scenario, unexpected behavior.

Let’s analyze the code below:

public void UpdateEmployeeStatus(Employee employee)
  {
    if (employee.Absences >= 3)
    {
      employee.Status = (int)EmployeeStatus.OnLeave;
    }
    else
    {
      employee.Status = (int)EmployeeStatus.Active;
    }
  _employeeRepository.Update(employee);
  _employeeRepository.SaveChanges();
}

Note that the name of the method above informs the reader that it updates the employee’s status, but it is doing something more than that—it is updating the status in the database and saving the changes. This is a good example of a side effect. The behavior of this method is not consistent with its function—that is, its name is a lie.

To get around this problem, we can refactor the code as follows:

public void UpdateEmployeeStatusClean(Employee employee)
{
  employee.Status = (int)GetEmployeeStatus(employee.Absences);
}

public EmployeeStatus GetEmployeeStatus(int absenteeismCount)
{
  if (absenteeismCount >= 3)
  {
    return EmployeeStatus.OnLeave;
  }
  else
  {
    return EmployeeStatus.Active;
  }
}

Now the GetEmployeeStatus method only determines the employee’s status based on the number of absences. It has no side effects, it just returns the calculated status. The UpdateEmployeeStatus method only defines the employee’s status based on the result of the GetEmployeeStatus method, without persisting the data as was done before the refactoring.

Data persistence can be handled elsewhere, perhaps in an application service or an infrastructure layer that specifically deals with storing and retrieving objects from the database. This way, the UpdateEmployeeStatus method no longer has any side effects, because responsibilities are well defined and there are no hidden behavior.

3. Writing Good Comments

Comments are important because they help with documentation and understanding of the code in any software project.

They provide additional information about how the code works, clarify the intent behind certain implementation decisions, help other developers understand difficult concepts, and enable the use of references such as copyright, etc.

Despite the advantages listed above, the misuse of comments can compromise the quality of code, resulting in dirty code and running the risk of confusing readers. To avoid this and other problems, we must follow some guidelines.

Don’t Write Comments to Make Up for Bad Code

It is very common to find comments trying to explain how poorly written code works. Many developers create confusing code, and instead of resolving the mess, they simply write a comment explaining the logic of that feature.

We all write bad code sometimes. Even programmers with a lot of experience, when creating logic, write bad code just to transpose their ideas and not interrupt their thinking trying to create good code. But right after creating the logic necessary, experienced programmers refactor the code creating something much better than the first version. This is the best thing to do.

We must explain ourselves through the code itself. Creating a comment explaining poorly written code will never be more productive than refactoring that code and leaving it clean, without the need for any comment.

Let’s see an example of bad code with comments:

// This method calculates an employee's salary deduction based on their category
public decimal CalculateSalaryDiscount(Employee employee)
{
  decimal discount = 0;

  if (employee.Category == 3)
  {
    // Category 3 has a 10% discount for salaries above $1000            
    discount = employee.Salary * 0.1m;
  }
  else if (employee.Category == 2)
  {
    // Category 2 has a fixed discount of $200
    discount = 200;
  }
  else if (employee.Category == 1)
  {
     // Category 1 has a fixed discount of $100
     discount = 100;
  }
  else
  {
    // Other categories do not have a discount
    discount = 0;
  }

  return discount;
}

The code above is poorly written, it has several if/else statements that make it difficult to read, in addition to loose numbers that in themselves do not explain anything. To get around this, several comments were added. Some of them are even unnecessary, like this one for example: // Other categories do not have a discount.

Now, see below the same code refactored in a way that there is no need for a single comment:

public decimal CalculateSalaryDiscount(Employee employee)
{
  decimal discount = 0;
  const decimal tenPercentDiscountRate = 0.1m;

  switch (employee.Category)
  {
    case (int)EmployeeCategory.Senior:
      discount = employee.Salary * tenPercentDiscountRate;
      break;
    case (int)EmployeeCategory.MidLevel:
      discount = 200;
      break;
    case (int)EmployeeCategory.Junior:
      discount = 100;
      break;
    default:
      break;
    }
  return discount;
}

Now let’s see what this code expresses. It brings a new constant called tenPercentDiscountRate which receives the value 0.1m. The name of the constant already demonstrates its function—to give a 10% discount when applied.

Then, all if/else statements were replaced by the switch statement. The use of the switch is often discussed in conversations about clean code, but in this context it is a good option, as it greatly reduces the use of conditionals. After adding the switch, an enum was created to represent the categories, such as Senior, Mid-level and Junior, which makes the code self-explanatory and highlights what each category means.

See how simple it can be to create code that does not require the use of comments, while expressing very well the idea behind each instruction or element present there?

And When Is It Not Possible to Avoid Using Comments?

We should avoid using comments as much as possible. However, unlike the example seen previously, in some scenarios, it is impossible to avoid them. And, in some situations, it is even advisable to use them.

A very common example of good comments are ones that are written for legal reasons like property rights:

// Copyright (C) 2024 by John Soft Smith, Inc. All rights reserved.
// Distributed under the terms of version 2 of the GNU General Public License
// For more information visit www.jssmithexample.com

Comments like this are welcome as they do not provide excessive detail on a subject. Instead, they provide subtle information and references to external links.

Comments That Save Time

Some comments are really useful as they bring warnings that can save hours and even more time than that. Note the example below:

// This test should not be run in production environments due to long execution time and high memory consumption
[Fact]
public void BackUpLargeFiles()
{
  List<string> records = new List<string>();
    
  for (int i = 0; i < 100000; i++)
  {
    records.Add($"Example {i}");
  }

  bool result = _serviceMock.WriteToFile(records);

  Assert.True(result);
}

The warning in this code informs the reader that this test should not be run in production environments as it takes a long time to run and can consume a large amount of resources. Warnings like this can prevent other developers from wasting time or causing future problems.

Despite being beneficial, comments like this should be temporary and should have a justifiable use.

TODO-type Comments

TODO tasks are an important resource that developers use to remember something they need to do in the code but cannot do at the current time. It could be a refactoring, creating new logic or even using a variable to debug a function.

Visual Studio has a tab to view all the TODOs present in the code. To view them, simply create a comment with the description // TODO and then click on the “View” submenu and then on “Task List”:

Visual Studio TODO Task

Although useful, TODO comments should also be temporary and should be removed as soon as possible. Code in development is likely to have TODO comments, but production code should not have any comments like this unless it is something very significant.

4. Creating Good Formatting

Code formatting can leave a good or bad impression. Poorly formatted code can leave the reader with the impression that little or no care was taken when creating it, leading them to believe that the functioning of the code must also be compromised.

It’s not enough for code to work and be complete. If it still needs to be formatted, it will look like it’s no good.

When we create code, we must think of a way to optimize it, but beyond that, we must worry about the impression we leave for those who will inspect it in the future.

Just like a sculptor who strives to impress the public with their art, we must also create code that impresses the reader, and that leaves them with the feeling that code is well written and easy to read—that is, it works well.

Here’s an example of poorly formatted code:

public void CalculateEmployeeSalaries(List<Employee> employees, Company company)
{
  try
  {
    foreach (var employee in employees)
    {
      decimal totalSalary = 0;

      if (employee.Status == (int)EmployeeStatus.Active)
      {
        for (int i = 0; i < employee.MonthsWorked; i++) { totalSalary += employee.Salary; }
        if (employee.Absences > 0)
        {
          totalSalary -= (employee.Salary / 30)
            * employee.Absences;
        }

        if (employee.Category == (int)EmployeeCategory.Junior)
        {
          totalSalary += (totalSalary * 0.1m);
        }

        if (company.National)
        {
          totalSalary += 1000;
        }
        employee.Salary = totalSalary;

        }
      }
    }
    catch (Exception ex)
    {
       _logger.LogError(ex.Message, "Error when calculating employee salaries");
    }
}

Note that the code above, despite being cohesive, has poor formatting. There are many blank lines where they should not exist, and there are places where there are no line breaks, while others have unnecessary line breaks.

Let’s see the same code using good formatting:

public void CalculateEmployeeSalaries(List<Employee> employees, Company company)
{
  try
  {
    foreach (var employee in employees)
    {
      if (employee.Status == (int)EmployeeStatus.Active)
      {
        decimal totalSalary = 0;
        
        for (int i = 0; i < employee.MonthsWorked; i++)
        {
          totalSalary += employee.Salary;
        }

        if (employee.Absences > 0)
        {
          totalSalary -= (employee.Salary / 30) * employee.Absences;
        }

        if (employee.Category == (int)EmployeeCategory.Junior)
        {
          totalSalary += (totalSalary * 0.1m);
        }

        if (company.National)
        {
          totalSalary += 1000;
        }

        employee.Salary = totalSalary;
      }
    }
  }
  catch (Exception ex)
  {
    _logger.LogError(ex.Message, "Error when calculating employee salaries");
  }
}

This new version of the CalculateEmployeeSalaries method is much more readable and gives a good impression because it has the line breaks where they should be and in the right amount. In addition, there are no unnecessary blank lines—they are organized in the right way.

It is important to remember that there is no convention regarding where to place blank lines and where not to—it is up to the developer to choose where to place them and where to avoid them. To achieve a good result, try writing the code and then review it to find points that can be improved. Another tip is to ask another developer to read your code. If they have a good impression, it means you are on the right path.

5. Avoiding Code Smells

“Code smell” is a term used in software engineering to describe code patterns that suggest the existence of underlying problems, such as design or implementation problems.

Just as an unpleasant smell can indicate the presence of something wrong in a physical environment, a code smell indicates something potentially problematic in the source code.

Code smells are not necessarily bugs or programming errors, but they serve as a warning that the code may be difficult to understand, modify or maintain. Clean code cannot have code smells—have you ever seen something clean that smells bad? Probably not. Identifying and correcting code smells is essential to obtain clean code.

There are many types of code smells, which can vary depending on the programming language, programming paradigm and project context.

Below we will analyze five of the main categories of code smells:

Code Smell 1: Bloaters

Bloaters are code smells that indicate that parts of the code are becoming too large. This may include classes or methods with many lines of code, excessively long lists or arrays, or even the presence of very large objects.

Note the class below:

public class EmployeeManagerService
{
  public void RegisterNewEmployee()
  {
   	//Rest of the code...
  }
	
  public void UpdateEmployee()
  {
    //Rest of the code...
  }
	
  public void DeleteEmployee()
  {
    //Rest of the code...
  }

  public void SendVacationNotificationToEligibleEmployees()
  {
    //Rest of the code...
  }

  private List<Notification> GetElegibleEmplyees()
  {
    //Rest of the code...
  }

  public decimal CalculateAbsenceDiscount(decimal absences, int paycheck)
  {
    //Rest of the code...
  }
	
  public void ProcessEmployeeRates()
  {
    //Rest of the code...
  }

  private IQueryable<Employee> GetEmployeesByNationality(bool isNational)
  {
    //Rest of the code...
  }

  private void CalculateEmployeesRates(IEnumerable<Employee> employees, bool isNational)
  {
   	//Rest of the code...
  }

  private void SendNotification(List<Employee> employees)
  {
    //Rest of the code...
  }
}

This is a bloated class. Note that the name of the class is EmployeeManagerService, which is a very comprehensive name. In addition, the class’s methods refer to various operations such as registering employees, sending notifications, calculating discounts and others.

To solve this problem, we can extract the code into different classes as you can see in the example below:

public class EmployeeRepository 
{
  public void RegisterNewEmployee()...

  public void UpdateEmployee()...

  public void DeleteEmployee()...
        
  private List<Notification> GetElegibleEmplyees()...

  private IQueryable<Employee> GetEmployeesByNationality(bool isNational)...
}

public class ProcessFinancialOperationsService
{
  public decimal CalculateAbsenceDiscount(decimal absences, int paycheck)...

  public decimal CalculateAbsenceDiscount(decimal absences, int paycheck)...
        
  public void ProcessEmployeeRates()...
        
  private void CalculateEmployeesRates(IEnumerable<Employee> employees, bool isNational)...
}
	
public class ProcessNotificationService
{
  public void SendVacationNotificationToEligibleEmployees()...
        
  private void SendNotification(List<Employee> employees)...
}

Now instead of having a single class with several methods, we have three classes, and each one has a defined responsibility within a scope—performing CRUD operations on the database, calculating financial values and processing notifications.

Remember to avoid creating bloated classes so that good code maintenance is possible. Whenever you notice that the class is bloated, think about refactoring it and extracting it into smaller classes.

Code Smell 2: Object-Orientation Abusers

This type of code smell occurs when object-oriented principles are being misused, such as classes that violate the single responsibility principle, excessive inheritance and classes that have too many responsibilities.

See the following example:

public class Employee
{
  public string Name { get; set; }
  public double Salary { get; set; }

  public virtual void Work()
  {
    Console.WriteLine($"{Name} is performing general work.");
  }
}

public class Manager : Employee
{
  public override void Work()
  {
    Console.WriteLine($"{Name} is managing projects.");
  }
}

public class Developer : Employee
{
  public override void Work()
  {
    Console.WriteLine($"{Name} is developing code.");
  }
}

public class Designer : Employee
{
  public override void Work()
  {
    Console.WriteLine($"{Name} is designing interfaces.");
  }
}

In this example, we are using inheritance to create different types of employees, such as managers, developers and designers. Each subclass extends the Employee class and overrides the Work() method to reflect the specific type of work performed by each type of employee.

However, this approach violates the single responsibility principle because the Employee class now has many responsibilities. Additionally, it can lead to a deep and complex class hierarchy as new types of employees are added, which can make the code difficult to understand and maintain.

A better approach would be to use composition instead of inheritance, separating the responsibilities into separate classes. Let’s implement this fix.

Below is a corrected approach using composition:

public interface IEmployeeBehavior
{
  void Work();
}

public class ManagerBehavior : IEmployeeBehavior
{
  public void Work()
  {
    Console.WriteLine("The manager is managing projects.");
  }
}

public class DeveloperBehavior : IEmployeeBehavior
{
  public void Work()
  {
    Console.WriteLine("The developer is writing code.");
  }
}

public class DesignerBehavior : IEmployeeBehavior
{
  public void Work()
  {
    Console.WriteLine("The designer is creating designs.");
  }
}

public class Employee
{
  private readonly IEmployeeBehavior behavior;

  public Employee(IEmployeeBehavior behavior)
  {
    this.behavior = behavior;
  }

  public void PerformWork()
  {
    behavior.Work();
  }
}

In this approach, we define an IemployeeBehavior interface that represents the generic behavior of an employee, in this case, just the Work() method. We then implement concrete classes, such as ManagerBehavior, DeveloperBehavior and DesignerBehavior, to represent the specific behaviors of each type of employee.

The Employee class now uses composition, receiving an object of type IemployeeBehavior in its constructor. This allows different behaviors to be dynamically injected into Employee instances, without the need for inheritance. The PerformWork() method calls the Work() method of the associated behavior.

This approach is more flexible and modular, allowing new types of employee behaviors to be added easily by creating new implementations of IemployeeBehavior without affecting existing code. Furthermore, it avoids problems such as the explosion of inheritance hierarchies and overgeneralized base classes.

Code Smell 3: Change Preventers

These code smells refer to highly coupled code snippets that make it difficult to modify the code in isolation. As examples, we can classify highly coupled classes or methods as having many dependencies and inappropriate use of design patterns that make the code inflexible.

Let’s see an example of change preventers:

public class Notification
{
  public string Message { get; set; }

  public void SendNotification()
  {
    //Logic to send notification
  }
}

public class NotificationService
{
  public void ProcessNotification(Notification notification)
  {
    if (!string.IsNullOrEmpty(notification.Message))
    {
       notification.SendNotification();
    }
    else
    {
       Console.WriteLine("Notification: No message to send.");
    }
  }
}

In this example, the NotificationService class is created to handle sending notifications. However, the ProcessSendingNotifications method checks whether the message is empty before sending the notification.

This is an example of a change preventer code smell, as it adds complexity to the code rather than relying on the Notification class’s sole responsibility for sending notifications. Instead, the code should simply call SendNotification() and let the Notification class handle the sending logic.

So to solve this problem we can refactor the code as follows:

public class Notification
{
  public string Message { get; set; }

  public void SendNotification()
  {
    if (!string.IsNullOrEmpty(Message))
    {
      //Logic to send notification
    }
    else
    {
       Console.WriteLine("Notification: No message to send.");
    }
  }
}

public void ProcessSendingNotifications(Notification notification)
{
   notification.SendNotification();
}

Now, the SendNotification() method in the Notification class is responsible for checking whether the message is empty before sending the notification. This eliminates unnecessary complexity in the ProcessSendingNotifications method code so that the Notification class is responsible for its internal logic.

Code Smell 4: Dispensables

This code smell refers to parts of the code that are not needed and can be removed without affecting the system’s behavior. Examples include duplicate code, obsolete comments and unused features.

Let’s see an example of dispensables.

public class ShoppingCart
{
  public void AddItem(string item)
  {
    Console.WriteLine("Item added to cart: " + item);
  }

  public void RemoveItem(string item)
  {
    Console.WriteLine("Item removed from cart: " + item);
  }

  public double CalculateTotal()
  {
     Console.WriteLine("Calculating total...");
     return 0.0;
  }

  public void PrintReceipt()
  {
    Console.WriteLine("Printing receipt...");
  }
}

public class ShoppingCartService
{
  public void ProcessShoppingCart(ShoppingCart cart)
  {
    cart.AddItem("Product 1");
    cart.AddItem("Product 2");
    cart.RemoveItem("Product 1");
    double total = cart.CalculateTotal();
    Console.WriteLine("Total: $" + total);
  }
}

In this example, the ShoppingCart class has methods for adding items to the cart, removing items from the cart, calculating the cart total and printing the purchase receipt. However, in the CalculateTotal and PrintReceipt methods, only the output messages are present and the logic to calculate the total and print the receipt is not implemented. It does not make any difference in this context. These unused features represent a dispensable code smell.

To fix it, we can simply remove these methods:

public class ShoppingCart
{
  public void AddItem(string item)
  {
    Console.WriteLine("Item added to cart: " + item);
  }

  public void RemoveItem(string item)
  {
    Console.WriteLine("Item removed from cart: " + item);
  }
}

public class ShoppingCartServiceBetter
{
  public void ProcessShoppingCart(ShoppingCart cart)
  {
    cart.AddItem("Product 1");
    cart.AddItem("Product 2");
    cart.RemoveItem("Product 1");
  }
}

Note that in this corrected version of the code, the CalculateTotal and PrintReceipt methods were removed from the ShoppingCart class, as they were not being used. Now the class has only the methods needed to add and remove items from the cart. This eliminates unnecessary code smells, making the code cleaner and easier to understand.

Code Smell 5: Couplers

This code smell occurs when there is a high dependency between different modules or components of the system. Examples include excessive coupling between classes, excessive communication between objects, and circular dependencies.

Below is a common example of couplers:

public class DataStorage
{
  public string GetData()
  {
    return "Some data from database";
  }
}

public class DataProcessor
{
  private DataStorage _dataStorage;

  public DataProcessor(DataStorage dataStorage)
  {
    _dataStorage = dataStorage;
  }

  public void ProcessData()
  {
    string data = _dataStorage.GetData();
    
    Console.WriteLine("Processing data: " + data);
  }
}

public class Client
{
  public void DoSomething()
  {
    DataStorage storage = new DataStorage();
    DataProcessor processor = new DataProcessor(storage);
      processor.ProcessData();
  }
}

public static void Main(string[] args)
{
  Client client = new Client();
  client.DoSomething();
}

In the code above, the DataProcessor class is tightly coupled to the DataStorage class because the DataProcessor receives an instance of DataStorage in its constructor. This means that any changes to the DataStorage implementation may require changes to the DataProcessor.

Ideally, classes should be independent and loosely coupled. One way to improve this would be to introduce a common interface between DataStorage and DataProcessor, allowing DataProcessor to be used with different data storage implementations without needing to be modified. This would make the code more flexible and easier to maintain.

Below is the same example, but this time without code smells.

public interface IDataStorage
{
  string GetData();
}

public class DatabaseStorage : IDataStorage
{
  public string GetData()
  {
    return "Some data from database";
  }
}

public class FileStorage : IDataStorage
{
  public string GetData()
  {
    return "Some data from file";
  }
}

public class DataProcessor
{
  private IDataStorage _dataStorage;

  public DataProcessor(IDataStorage dataStorage)
  {
    _dataStorage = dataStorage;
  }

  public void ProcessData()
  {
    string data = _dataStorage.GetData();
    
    Console.WriteLine("Processing data: " + data);
  }
}

public class Client
{
  public void DoSomething()
  {
    IDataStorage storage = new DatabaseStorage(); // or FileStorage()
    DataProcessor processor = new DataProcessor(storage);
    processor.ProcessData();
  }
}

public class Program
{
  public static void Main(string[] args)
  {
    Client client = new Client();
    client.DoSomething();
  }
}

In the example above, we introduced an IdataStorage interface, which defines a contract to get data. We then implement this interface in two separate classes: DatabaseStorage and FileStorage, each dealing with getting data from a specific location (database or file, respectively).

The DataProcessor class now only depends on the IdataStorage interface, rather than a specific data storage implementation. This reduces the coupling between the DataProcessor and the concrete classes that provide the data, making the code more flexible and easier to maintain.

In the Client class, we can instantiate DataProcessor with any desired IdataStorage implementation without the need to modify DataProcessor. This promotes code reuse and simplifies the introduction of new types of data storage into the system, in addition to removing code smell couplers.

Conclusion

In this post, we had an introduction to clean code, saw what this term means and covered five of the main topics related to the subject, in addition to giving a practical example of each of them.

It is important to note that writing clean code requires a lot of dedication—after all, there are many points to consider when creating good code.

For full coverage of topics related to clean code, I suggest reading the famous book written by Robert C. Martin—Clean Code: A Handbook of Agile Software Craftsmanship.


assis-zang-bio
About the Author

Assis Zang

Assis Zang is a software developer from Brazil, developing in the .NET platform since 2017. In his free time, he enjoys playing video games and reading good books. You can follow him at: LinkedIn and Github.

Related Posts

Comments

Comments are disabled in preview mode.