DEV Community

Matheus Gomes 👨‍💻
Matheus Gomes 👨‍💻

Posted on • Updated on

Food review, refactoring a long method

Refactoring

Let`s think about code as food when we prepare our food we make it delicious at the beginning, we make it shine and we are proud of it, but, when we receive guests, we try to use different ingredients so everyone can enjoy it. So we add some spice here and there, some oil and other things. We finish the preparation and serve the food, when our guests are eating they realize something is not right, what started as a lasagna ended up being spaghetti and isn't smelling good, actually, is smelling pretty bad.

Comparing to our code, we call that a code smell, it's when we have bloaters, which are code, methods, and classes that increased to such gargantuan proportions that they are hard to work with.

Usually, these smells do not crop up right away, rather they accumulate over time as the program evolves (and especially when nobody makes an effort to eradicate them). We solve this problem by refactoring the code, or, in food terms, removing some ingredients of the dish. This refactoring makes the code run faster and more efficiently, making it easier to adapt and add features.

Well, let`s see some methods to refactor. Starting up with the problem of a...

Long Method

A long method is when a method contains too many lines of code. A good method usually contains less than 10 lines (in most cases).

How to solve?

Solution #1 - Extract method

❌Problem

You have a code fragment that can be grouped together

void PrintOwing() 
{
  this.PrintBanner();

  // Print details.
  Console.WriteLine("name: " + this.name);
  Console.WriteLine("amount: " + this.GetOutstanding());
}
Enter fullscreen mode Exit fullscreen mode

✔Solution

Move this code to a separate new method (or function) and replace the old code with a call to the method.

void PrintOwing()
{
  this.PrintBanner();
  this.PrintDetails();
}

void PrintDetails()
{
  Console.WriteLine("name: " + this.name);
  Console.WriteLine("amount: " + this.GetOutstanding());
}
Enter fullscreen mode Exit fullscreen mode

This makes the code more readable and makes a method specific to one action, being possible to re-utilize it.

Solution #2 - Reduce Local Variables and Parameters Before Extracting a Method

If local variables and parameters interfere with extracting a method, use Replace Temp with Query, Introduce Parameter Object, or Preserve Whole Object.

Using Replace Temp with Query

❌Problem

You place the result of an expression in a local variable for later use in your code.

double CalculateTotal() 
{
  double basePrice = quantity * itemPrice;

  if (basePrice > 1000) 
  {
    return basePrice * 0.95;
  }
  else 
  {
    return basePrice * 0.98;
  }
}
Enter fullscreen mode Exit fullscreen mode

✔Solution

Move the entire expression to a separate method and return the result from it. Query the method instead of using a variable. Incorporate the new method in other methods, if necessary.

double CalculateTotal() 
{
  if (BasePrice() > 1000) 
  {
    return BasePrice() * 0.95;
  }
  else 
  {
    return BasePrice() * 0.98;
  }
}
double BasePrice() 
{
  return quantity * itemPrice;
}
Enter fullscreen mode Exit fullscreen mode

Almost the same principle as before, it sure makes the code looks clean and more readable despite adding more lines to it.

Using Introduce Parameter Object

❌Problem

Your methods contain a repeating group of parameters.

int CalculateSomething(int firstValue, int secondValue, int thirdValue);
int CalculateSomethingMore(int firstValue, int secondValue, int thirdValue);
int CalculateAnything(int firstValue, int secondValue, int thirdValue);
Enter fullscreen mode Exit fullscreen mode

✔Solution

Replace these parameters with an object.

int CalculateSomething(valuesObj);
int CalculateSomethingMore(valuesObj);
int CalculateAnything(valuesObj);
Enter fullscreen mode Exit fullscreen mode

This way we don't need to understand what it does in the call, we leave this description to the method!

Preserve Whole Object

❌Problem

You get several values from an object and then pass them as parameters to a method.

int low = daysTempRange.GetLow();
int high = daysTempRange.GetHigh();
bool withinPlan = plan.WithinRange(low, high);
Enter fullscreen mode Exit fullscreen mode

✔Solution

Instead, try passing the whole object.

bool withinPlan = plan.WithinRange(daysTempRange);
Enter fullscreen mode Exit fullscreen mode

Same as before, we need to leave the description and handling to the method inside, not the caller.

Solution #3 - Replace Method with Method Object

If none of the previous recipes help, try moving the entire method to a separate object via Replace Method with Method Object.

❌Problem

You have a long method in which the local variables are so intertwined that you can't apply Extract Method.

public class Order 
{
  // ...
  public double Price() 
  {
    double primaryBasePrice;
    double secondaryBasePrice;
    double tertiaryBasePrice;
    // Perform long computation.
  }
}
Enter fullscreen mode Exit fullscreen mode

✔Solution

Transform the method into a separate class so that the local variables become fields of the class. Then you can split the method into several methods within the same class.

public class Order 
{
  // ...
  public double Price() 
  {
    return new PriceCalculator(this).Compute();
  }
}

public class PriceCalculator 
{
  private double primaryBasePrice;
  private double secondaryBasePrice;
  private double tertiaryBasePrice;

  public PriceCalculator(Order order) 
  {
    // Copy relevant information from the
    // order object.
  }

  public double Compute() 
  {
    // Perform long computation.
  }
}
Enter fullscreen mode Exit fullscreen mode

Solution #4 - Conditionals and Loops

Conditional operators and loops are a good clue that code can be moved to a separate method. For conditionals, use Decompose Conditional. If loops are in the way, try Extract Method.

Decompose Conditional

❌Problem

You have a complex conditional (if-then/else or switch).

if (date < SUMMER_START || date > SUMMER_END) 
{
  charge = quantity * winterRate + winterServiceCharge;
}
else 
{
  charge = quantity * summerRate;
}
Enter fullscreen mode Exit fullscreen mode

✔Solution

Decompose the complicated parts of the conditional into separate methods: the condition, then and else.

if (isSummer(date))
{
  charge = SummerCharge(quantity);
}
else 
{
  charge = WinterCharge(quantity);
}
Enter fullscreen mode Exit fullscreen mode

Extract Method

Conditional operators and loops are a good clue that code can be moved to a separate method. For conditionals, use Decompose Conditional. If loops are in the way, try Extract Method.

❌Problem

You have a code fragment that can be grouped together.

void printProperties(IList users) 
{
  for (int i = 0; i < users.size(); i++)
  {
    string result = "";
    result += users.get(i).getName();
    result += " ";
    result += users.get(i).getAge();
    Console.WriteLine(result);

    // ...
  }
}
Enter fullscreen mode Exit fullscreen mode

✔Solution

Move this code to a separate new method (or function) and replace the old code with a call to the method.

void printProperties(IList users) 
{
  foreach (User user in users)
  {
    Console.WriteLine(getProperties(user));

    // ...
  }
}

string getProperties(User user)  
{
  return user.getName() + " " + user.getAge();
}
Enter fullscreen mode Exit fullscreen mode

So remember, make your food as Gordon is watching you, same with code, imagine Uncle Bob is watching you.

That`s it, folks, I guess this is some solutions to solve problems with long methods. This article was made after I started a course about refactoring in the refactoring.guru which is a great course, so go check it out to learn more!

Discussion (0)