Skip to main content

Inappropriate coding practices - Unused properties

Need

Elimination of unused properties in the application code

Context

  • Usage of C# 7.0 for modern language features and enhancements
  • Usage of Microsoft.AspNetCore.Mvc for building web applications using the ASP.NET Core MVC framework
  • Usage of Microsoft.EntityFrameworkCore for database access and management in .NET applications

Description

Non compliant code

public class Employee
{
public int Id { get; set; }
public string Name { get; set; }
public string Department { get; set; }
public string UnusedProperty1 { get; set; }
public string UnusedProperty2 { get; set; }
}

public class EmployeeController : Controller
{
private readonly ApplicationDbContext _context;

public EmployeeController(ApplicationDbContext context)
{
_context = context;
}

public IActionResult Index()
{
var employees = _context.Employees.Select(e => new
{
e.Id,
e.Name,
e.Department
}).ToList();

return View(employees);
}
}

In the above code, the Employee class defines two properties, UnusedProperty1 and UnusedProperty2, which are not used anywhere in the application. They are not used in any method in the EmployeeController, nor are they used in any views or models. These properties add unnecessary complexity to the code and may lead to performance issues.

The EmployeeController's Index method selects only the Id, Name, and Department properties from the Employee entities in the database. The UnusedProperty1 and UnusedProperty2 properties are not selected or used in any way, making them unnecessary.

This issue can lead to confusion for developers working on the code, as they may not know whether these properties are important or not. Additionally, it can lead to wasted resources, as the application may allocate memory for these properties even though they are not used.

In terms of security, while this issue does not directly lead to any known vulnerabilities, it is still a bad practice as it can make the code harder to maintain and understand, increasing the likelihood of bugs and vulnerabilities being introduced in the future.

Steps

  • Identify and remove the unused properties in the Employee class.
  • Ensure that the removal of the unused properties does not impact any other parts of the application.
  • Update the database schema or migration scripts to reflect the changes made to the Employee class.
  • Update any other code that interacts with the Employee class to remove references to the unused properties.
  • Test the application to ensure that it functions correctly after removing the unused properties.

Compliant code

public class Employee
{
public int Id { get; set; }
public string Name { get; set; }
public string Department { get; set; }
}

public class EmployeeController : Controller
{
private readonly ApplicationDbContext _context;

public EmployeeController(ApplicationDbContext context)
{
_context = context;
}

public IActionResult Index()
{
var employees = _context.Employees.Select(e => new
{
e.Id,
e.Name,
e.Department
}).ToList();

return View(employees);
}
}

The unused properties UnusedProperty1 and UnusedProperty2 have been removed from the Employee class. This reduces unnecessary complexity in the source code and optimizes the application's performance.

The EmployeeController class, which interacts with the Employee class, has also been updated to reflect these changes. The Index method, which retrieves a list of employees, no longer includes the unused properties in its selection.

Before deploying these changes, ensure that the removal of these properties does not impact any other parts of the application. Update the database schema or migration scripts to reflect the changes made to the Employee class. Test the application thoroughly to ensure that it functions correctly after the removal of the unused properties.

References