Three Changes You Should Always Make Before Checking In Any Code

Photo by Maico Amorim

This story originally appeared in Hacker Noon on March 11, 2017.

A code refactor always leaves me with a feeling of accomplishment. Although major refactorings are the most satisfying, every project doesn't always warrant them. Here are 3 quick and easy refactorings that I make to all of my projects in order to improve code readability:

1. Clean up formatting

The overall format of your code is what makes it possible to quickly navigate to areas of interest. Consistent indentation, line breaks, and patterns help programmers skim large chunks of code. Take the following sloppily formatted code for example:

Inventory inventory = new Inventory();
for (int i = 0; i < cars.Count; i++){
    inventory.Cars.Add(cars[i]);
    var owner = owners.Where(x => x.VIN == cars[i].VIN).OrderByDescending(x => x.PurchaseDate).FirstOrDefault();
inventory.PreviousOwners.Add(new Owner { VIN = cars[i].VIN,
                                        Name = owner.Name});
}

and compare it to this:

Inventory inventory = new Inventory();
for (int i = 0; i < cars.Count; i++)
{
    inventory.Cars.Add(cars[i]);
    var owner = owners.Where(x => x.VIN == cars[i].VIN)
                        .OrderByDescending(x => x.PurchaseDate)
                        .FirstOrDefault();
inventory.PreviousOwners.Add(new Owner 
                                        {
                                        VIN = cars[i].VIN,
                                        Name = owner.Name
                                        });
}

The second example above consistently indents lines, adds new lines, and follows consistent coding patterns. This makes it easy to skim the code quickly.

Books have chapters, headings, and paragraphs defined by formatting that make it easy to find what is needed at a glance?—?formatting code makes it possible to find things easily too.

2. Rename classes, methods, and variables

Classes, methods, and variables should be named in such that they help the programmer understand what is happening in the code. The shorter the scope of an object the more permissible it is to use shorter names (eg. "i" as a counter in a loop that's only a line or two long).

It's easy to use uninformative names when writing a "first draft" of your program, but using the first name that comes to mind isn't always the best choice. Take a look at the following example:

public IEnumerable<string> GetData(int year)
{
    var result = CallApi("/Cars", year);
IEnumerable<string> output = new IEnumerable<string>();
foreach(var c in result)
    {
        output.Add(c.Make);
    }
}

versus:

public IEnumerable<string> RetrieveCarMakes(int year)
{
    var inventory = CallApi("/Cars", year);
IEnumerable<string> carMakes = new IEnumerable<string>();
    foreach(var car in inventory)
    {
        carMakes.Add(car.Make);
    }
}

Using names that make sense make it much easier for someone else (or your future-self) to figure out what your code is doing.

3. Break up long expressions

When you get into a code writing groove it's easy to keep chaining commands together or using single-line syntax to speed up your writing. Often times, I look back on my code a day later and I see long expressions similar to this. Take a look at this two line expression:

bool hasHighSaleProbability = (daysOnLot < 60) ? true : (color == "Red" ? true : false);
var highSaleProbabilityVehicles = Inventory.Where(x => x.DaysOnLot < 60 or x.Color == "Red").Select(x => new { Make = x.Make, Model = x.Model, Year = x.Year }).Distinct().Select(x => new RecentInventoryView { YearDropdown = new SelectListItem { Text = x.Year, Value = x.Year }, MakeModelDropdown = new SelectListItem { Text = x.Make + " " + x.Model, Value = x.Make + " " + x.Model } });

Compared against this expression that has been broken out across multiple lines:

bool hasHighSaleProbability = false;
if (daysOnLot < 60 || color == "Red")
{
    hasHighSaleProbability = true;
}
var distinctMakesModelsYears = Inventory
    .Where(x => x.DaysOnLot < 60 or x.Color == "Red")
    .Select(x => new 
    { 
    Make = x.Make, 
    Model = x.Model, 
    Year = x.Year 
    })
    .Distinct()
    .ToList();
var recentInventoryView = distinctMakesModelsYears
    .Select(x => new RecentInventoryView 
    { 
        YearDropdown = new SelectListItem 
        { 
        Text = x.Year, 
        Value = x.Year 
        }, 
        MakeModelDropdown = new SelectListItem 
        { 
        Text = x.Make + " " + x.Model, 
        Value = x.Make + " " + x.Model 
        } 
    });

Although the first code snippet is technically shorter and has fewer lines, it is nearly unreadable. The second snippet breaks out the the if logic and breaks up all of the chained methods into more logical variables. The result? Code logic that is much easier to follow.