Maintaining Good Code

Stephanus Susanto | July 31st, 2017

Do you ever look back at your code to see if it’s good enough or not? Do you ever have trouble reading and understanding your own code?

We know that every time we write code, there will be someone that needs to read it in the future. It’s not a great start if you’re not able to read your own code after only a few days. Writing code is easier than reading it. But writing messy code is as easy as writing good code, the differences are messy code is tougher to read and will inject technical debt into your project that can be hard to recover from.

We can say that our code is good if it’s able to self-document.

In this post, we will go through how to improve code maintainability using the sample code below. This sample is written in C#, but the principles in this post can apply to code written in any language.


public double GetTotalPrice(Customer customer)
{
    var dcList = new List { "Fashion", "Food" };
    var dmList = new List { "X Electronic" };
    var d = 0;
    var ok = false;
    var y = DateTime.Now.Year - customer.JoinDate.Year;
    if (DateTime.Now.DayOfYear < customer.JoinDate.DayOfYear) { y = y - 1; } //// give discount if customer joined more than 1 year if (y > 1)
    {
        ok = true;
    }

    //// give discount if customer is gold member
    if (customer.IsGoldCustomer)
    {
        ok = true;
    }

    if (Product != null)
    {
        if (ok)
        {
            if (Quantity > 0)
            {
                if (Quantity >= 50)
                {
                    d = 20;
                }
                else if (Quantity >= 20)
                {
                    d = 10;
                }
                else if (Quantity >= 10)
                {
                    d = 5;
                }

                foreach (var dc in dcList)
                {
                    if (dc.Equals(Product.Category))
                    {
                        d += 5;
                        break;
                    }
                }

                foreach (var dc in dmList)
                {
                    if (dc.Equals(Product.Manufacturer))
                    {
                        d += 10;
                        break;
                    }
                }
            }
            else
            {
                throw new Exception("Quantity should be more than 0");
            }
        }
    }
    else
    {
        throw new NullReferenceException("Product can not be null.");
    }

    if (d > 25)
    {
        d = 25;
    }

    return (Product.Price * Quantity) - (d * Product.Price * Quantity) / 100;
}

You can check the code metrics on visual studio to see the complexity of your code.

code metric visual studio complexity

This is the code metric result for the code above.

code metric result visual studio complexity

The maintainability index got only a 43, and the other stats, complexity and class coupling, are high. We will improve this.

The code should handle these rules for getting the price for the product:

  1. There will be a discount if the member has joined for more than 1 year or is a gold member.
  2. Discount will be calculated by the quantity of the purchased products.
    • If quantity ≥50, customers will get a 20% discount.
    • From 20 to 49, customers receive a 10% discount.
    • From 10 to 19, there is a 5% discount.
    • At <10, there is no discount.
  3. Fashion and food category items will get a 5% discount.
  4. X Electronic manufacturer will get a 10% discount.
  5. Maximum discount is 25%

Here are some principles you can use to improve the sample code.

Naming

  • Naming should be meaningful and describe the contents or functionality. We will refactor this code and make the variable names more meaningful.
  • Don’t use abbreviation if you can, it will create confusion for readers. Meaningful names should be applied to variables, methods/functions, classes, etc.
  • Where there is abbreviation and no clear naming, we will assign new naming:

public double GetTotalPrice(Customer customer)
{
    var discountCategories = new List { "Fashion", "Food" };
    var discountManufacturers = new List { "X Electronic" };
    var discount = 0;
    var IsMeetGetDiscountRule = false;
    var customerJoinedYearLength = DateTime.Now.Year - customer.JoinDate.Year;

    ...
}

Magic Number

  • We should remove any magic numbers. The number we put in the code should have context.
  • For example:
    • There is a ‘1’ when checking joined year date length of the customer. This will become “minJoinedYearLengthGetDiscount”.
    • There is also a ‘25’, which we will define as “maxDiscount” for customers.

public double GetTotalPrice(Customer customer)
{
    ...

    const int minJoinedYearLengthGetDiscount = 1;
    if (customerJoinedYearLength > minJoinedYearLengthGetDiscount)
    {
        IsMeetGetDiscountRule = true;
    }

    ...

    const int maxDiscount = 25;
    if (discount > maxDiscount)
    {
        discount = maxDiscount;
    }
    
    ...
}

Return Early/Fail First

  • Return early and fail first help us in eliminating deep indentation in our code. We will just return or fail if there is nothing more needed to do on the code. You can see the original code has deep indentation.
  • We will make the code throw an exception if the quantity is 0 or the product is null in the beginning.
  • We also just return the total price without the discount if the discount rules are not met.

public double GetTotalPrice(Customer customer)
{
    if (Product == null)
    {
        throw new NullReferenceException("Product can not be null.");
    }

    if (Quantity == 0)
    {
        throw new Exception("Quantity should be more than 0");
    }

    var discountCategories = new List { "Fashion", "Food" };
    var discountManufacturers = new List { "X Electronic" };
    var discount = 0;
    var IsMeetGetDiscountRule = false;

    var customerJoinedYearLength = DateTime.Now.Year - customer.JoinDate.Year;
    if (DateTime.Now.DayOfYear < customer.JoinDate.DayOfYear) { customerJoinedYearLength = customerJoinedYearLength - 1; } const int minJoinedYearLengthGetDiscount = 1; //// give discount if customer joined more than 1 year if (customerJoinedYearLength > minJoinedYearLengthGetDiscount)
    {
        IsMeetGetDiscountRule = true;
    }

    //// give discount if customer is gold member
    if (customer.IsGoldCustomer)
    {
        IsMeetGetDiscountRule = true;
    }

    if (IsMeetGetDiscountRule == false)
    {
        return (Product.Price * Quantity);
    }

    if (Quantity >= 50)
    {
        discount = 20;
    }
    else if (Quantity >= 20)
    {
        discount = 10;
    }
    else if (Quantity >= 10)
    {
        discount = 5;
    }

    foreach (var discountCategory in discountCategories)
    {
        if (discountCategory.Equals(Product.Category))
        {
            discount += 5;
            break;
        }
    }

    foreach (var discountManufacture in discountManufacturers)
    {
        if (discountManufacture.Equals(Product.Manufacturer))
        {
            discount += 10;
            break;
        }
    }

    const int maxDiscount = 25;
    if (discount > maxDiscount)
    {
        discount = maxDiscount;
    }

    return (Product.Price * Quantity) - (discount * Product.Price * Quantity) / 100;
}

Mayfly Variables

  • Variables should initialize late, close to where they will be used. Variables should have a short lifespan, so the reader will read it more easily and only have to remember it for a short time.

public double GetTotalPrice(Customer customer)
{
    if (Product == null)
    {
        throw new NullReferenceException("Product can not be null.");
    }

    if (Quantity == 0)
    {
        throw new Exception("Quantity should be more than 0");
    }

    var customerJoinedYearLength = DateTime.Now.Year - customer.JoinDate.Year;
    if (DateTime.Now.DayOfYear < customer.JoinDate.DayOfYear) { customerJoinedYearLength = customerJoinedYearLength - 1; } const int minJoinedYearLengthGetDiscount = 1; var IsMeetGetDiscountRule = false; //// give discount if customer joined more than 1 year if (customerJoinedYearLength > minJoinedYearLengthGetDiscount)
    {
        IsMeetGetDiscountRule = true;
    }

    //// give discount if customer is gold member
    if (customer.IsGoldCustomer)
    {
        IsMeetGetDiscountRule = true;
    }

    if (IsMeetGetDiscountRule == false)
    {
        return (Product.Price * Quantity);
    }

    var discount = 0;
    if (Quantity >= 50)
    {
        discount = 20;
    }
    else if (Quantity >= 20)
    {
        discount = 10;
    }
    else if (Quantity >= 10)
    {
        discount = 5;
    }

    var discountCategories = new List { "Fashion", "Food" };
    foreach (var discountCategory in discountCategories)
    {
        if (discountCategory.Equals(Product.Category))
        {
            discount += 5;
            break;
        }
    }

    var discountManufacturers = new List { "X Electronic" };
    foreach (var discountManufacture in discountManufacturers)
    {
        if (discountManufacture.Equals(Product.Manufacturer))
        {
            discount += 10;
            break;
        }
    }

    const int maxDiscount = 25;
    if (discount > maxDiscount)
    {
        discount = maxDiscount;
    }

    return (Product.Price * Quantity) - (discount * Product.Price * Quantity) / 100;
}

Function/Method

  • Break complex functions into separate functions. Using functions, we follow the “don’t repeat yourself” (DRY) principle. We will avoid duplication and organize the logic into small pieces. Good functions should do just one thing.
  • In this case, we will break the code into these functions with explanatory names:
    • ValidateData()
    • GetDiscount()
    • GetDiscountFromQuantities()
    • GetDiscountFromCategories()
    • GetDiscountFromManufacturers()
    • IsMeetGetDiscountRule(Customer customer)
    • GetCustomerJoinedYearLength(DateTime joinDate)
  • We can also remove comments because they are already described in the function names.

public double GetTotalPrice(Customer customer)
private double TotalPriceBeforeDiscount => Product?.Price * Quantity ?? 0;

private double TotalPriceWithDiscount
{
	get
	{
		var discount = GetDiscount();
		return TotalPriceBeforeDiscount - (discount * TotalPriceBeforeDiscount) / 100;
	}
}

public double GetTotalPrice(Customer customer)
{
	ValidateData();
	
	return IsMeetGetDiscountRule(customer) == false ? TotalPriceBeforeDiscount : TotalPriceWithDiscount;
}

private void ValidateData()
{
	if (Product == null)
	{
		throw new NullReferenceException("Product can not be null.");
	}

	if (Quantity == 0)
	{
		throw new Exception("Quantity should be more than 0");
	}
}

private double GetDiscount()
{
	const int maxDiscount = 25;

	var discount = 0;

	discount += GetDiscountFromQuantities();
	discount += GetDiscountFromCategories();
	discount += GetDiscountFromManufacturers();

	return discount > maxDiscount ? maxDiscount : discount;
}

private int GetDiscountFromQuantities()
{
	if (Quantity >= 50)
	{
	   return 20;
	}

	if (Quantity >= 20)
	{
		return 10;
	}

	return Quantity >= 10 ? 5 : 0;
}

private int GetDiscountFromCategories()
{
	var discountCategories = new List { "Fashion", "Food" };
	return discountCategories.Contains(Product.Category) ? 5 : 0;
}

private int GetDiscountFromManufacturers()
{
	var discountManufacturers = new List { "X Electronic" };
	return discountManufacturers.Contains(Product.Manufacturer) ? 10 : 0;
}

private bool IsMeetGetDiscountRule(Customer customer)
{
	const int minJoinedYearLengthGetDiscount = 1;

	return GetCustomerJoinedYearLength(customer.JoinDate) > minJoinedYearLengthGetDiscount || customer.IsGoldCustomer;
}

private int GetCustomerJoinedYearLength(DateTime joinDate)
{
	if (DateTime.Now.DayOfYear >= joinDate.DayOfYear)
	{
		return DateTime.Now.Year - joinDate.Year;
	}

	return DateTime.Now.Year - joinDate.Year - 1;
}

You can see the full code here. There will be an original folder and a refactored folder.

After our updates, the code metric shows significant improvement. We see the maintainability index jump to 79 and the other stats drop to normal levels.

code metric result visual studio improved

Other Improvements

There are some other improvements we can make.

  • Break the code into separate classes
  • Create a discount class(es)
  • Use table-driven design for the quantity discount

But already we can see the significant difference between the original code and the refactored code. It is easier to read and understand the code.

Having code reviews or using paired programming can help in avoiding the creation of bad code. Doing your own code review ensures maintainability and readability of the code because you must be able to read and understand your own code!

Uncle Bob reminds us of the Boy Scout Rule, “Leave the campground cleaner than the way you found it.” In the case of your code, always go back for a second look to see if there is any opportunity to clean it up before checking it into your project.

Subscribe

* indicates required
Stephanus Susanto

Stephanus is the Regional Director for Palador Indonesia. He has written a book on app development for Android.