How to solve a Code Review Exercise - Using SOLID Principles

Solving a code review exercise is meant to judge your coding skills as well how you architect the code.

Refactor the Piece of Code

The interviewer will give you a simple code like the one below and ask you to refactor the code for better readability and scalability.

import java.util.Date

enum class ExpenseType {
    DINNER, BREAKFAST, CAR_RENTAL
}

class Expense {
    lateinit var type: ExpenseType
    var amount: Int = 0
}

class ExpenseReport {
    fun printReport(expenses: List<Expense>) {
        var total = 0
        var mealExpenses = 0

        println("Expenses ${Date()}")

        for (expense in expenses) {
            if (expense.type == ExpenseType.DINNER || expense.type == ExpenseType.BREAKFAST) {
                mealExpenses += expense.amount
            }

            var expenseName = ""
            when (expense.type) {
                ExpenseType.DINNER -> expenseName = "Dinner"
                ExpenseType.BREAKFAST -> expenseName = "Breakfast"
                ExpenseType.CAR_RENTAL -> expenseName = "Car Rental"
            }

            val mealOverExpensesMarker = if (expense.type == ExpenseType.DINNER && expense.amount > 5000 || expense.type == ExpenseType.BREAKFAST && expense.amount > 1000) "X" else " "

            println(expenseName + "\t" + expense.amount + "\t" + mealOverExpensesMarker)

            total += expense.amount
        }

        println("Meal expenses: $mealExpenses")
        println("Total expenses: $total")
    }
}

The above code is used to calculate total expenses and meal expenses in case of expense type is Breakfast or Dinner.

How to identify the Problem in the above code :

Key areas where a code can be improved

SOLID Principles: Find which SOILD principle a class is violating.

• Single responsibility: ExpenseReport is calculating Total Expenses and Meal Expenses, which certainly violates the Single Responsibility Principle, Expense Report should not do the calculation part, it should do only the reporting part. So we can take out the responsibility of calculating total and meal expenses from the ExpenseReport class.

• Open/Closed Principle: ExpenseType and Expense Class are used to identify different types of expenses with the amount, here we can use some concrete classes for different Expense Types eg. DinnerExpense, CarRental Expenses.

• Interface Segregation Principle :

  Create a class ExpenseService which will calculate total and meal expenses.

  interface ExpenseReportInterface {
  	int calculateMealExpense(List<ExpenseInterface> expense);
  	int calculateTotalExpense(List<ExpenseInterface> expense);
  }

public class ExpenseReporterService implements ExpenseReportInterface {

@Override
public int calculateMealExpense(List<ExpenseInterface> expense) {
  int mealExpenses = 0;
  for (int i = 0; i < expense.size(); i++) {
      ExpenseInterface expenseObj = expense.get(i);
	if (expenseObj instanceof MealExpense) {
	    mealExpenses +=  expenseObj.calculateExpense();
	    }
	 }
	 return mealExpenses;
	}

@Override
public int calculateTotalExpense(List<ExpenseInterface> expense) {
   int total = 0;
   for (int i = 0; i < expense.size(); i++) {
	ExpenseInterface expenseObj = expense.get(i);
        total += expenseObj.calculateExpense();
           }
	return total;
	}
}

• Liskov Substitution Principle: Expense classes have different logic to calculate the expenses based on their type yet we can treat them as ExpenseType.

  so let’s fix this by using interfaces :

  public interface ExpenseInterface {
  	public int calculateExpense();
  	public String expenseName();
  }

  public interface MealExpense extends ExpenseInterface{
  	public int calculateMealExpense();
  	public String getMealOverExpense();	
  }

  We can use a Factory class to create different types of Expense Objects based on Type.

  Now DinnerExpense and Breakfast Expense will be the type of MealExpense and CarRentalExpense will be the type of ExpenseInterface yet all three are the same type of ExpenseInterface and can be treated as the same type.

  

  After Refactoring my Component diagram looks like above.

  Full Source code is available here in my GitHub repository, feel free to share your thoughts.