Intro Refactoring

Content

  • Fundamentals

  • How to change legacy code?

  • Tools

  • Techniques

We all have good intentions

But…​

Legacy code

What is legacy code?

  • "Others" code?

  • Code difficult to understand?

  • Difficult code to maintain?

  • …​

Legacy code is code without tests.

(2004) M. Feather

Reasons to change a software

  1. Add a feature

  2. Fix a bug

  3. Improve the design

  4. Optimize the use of resources

Adding a Feature

Fixing a Bug

Refactoring

Optimizing

Structure

Changes

Changes

Changes

 — 

Functionality

Changes

Changes

 — 

 — 

Resource Usage

 — 

 — 

 — 

Changes

Risks of change

  1. What changes do we have to make?

  2. How will we know that the changes were made correctly?

  3. How will we know that we have not broken anything?

Edit and Guess

Industry-standard?

Cover and Modify

Tests

Many dependencies?

The legacy code dilemma

When we change the code, we should have tests in place. For testing, we often have to change the code.

Breaking dependencies

The algorithm for changing legacy code

  1. Identify points of change.

  2. Find test points.

  3. Break dependencies.

  4. Write tests.

  5. Make changes and refactor.

Tools

  • IDEs

  • Unit Test Frameworks

Techniques

I don’t have much time, and I need to change the code

Sprout method

public class TransactionGate {
  public void postEntries(List entries) {
    for (Iterator it = entries.iterator(); it.hasNext(); ) {
      Entry entry = (Entry)it.next();
      entry.postDate();
    }
    transactionBundle.getListManager().add(entries);
  }
  //...
}

Agregar feature

Verify new entries are not previously in transactionBundle

Can be implemented "easily" with

public class TransactionGate {
  public void postEntries(List entries) {
    List entriesToAdd = new LinkedList();
    for (Iterator it = entries.iterator(); it.hasNext(); ) {
      Entry entry = (Entry)it.next();
      if (!transactionBundle.getListManager().hasEntry(entry) {
        entry.postDate();
        entriesToAdd.add(entry);
      }
    }
    transactionBundle.getListManager().add(entriesToAdd);
  }
  //...
}

But…​ we fall back into the legacy code loop. Let’s now try the Sprout method.

Extracting logic to the method

public class TransactionGate {
//...
  List uniqueEntries(List entries) {
    List result = new ArrayList();
    for (Iterator it = entries.iterator(); it.hasNext(); ) {
      Entry entry = (Entry)it.next();
      if (!transactionBundle.getListManager().hasEntry(entry) {
          result.add(entry);
      }
    }
    return result;
  }
//...
}

Built-in functionality

public class TransactionGate{
//...
  public void postEntries(List entries) {
    List entriesToAdd = uniqueEntries(entries);
    for (Iterator it = entriesToAdd.iterator(); it.hasNext(); ) {
      Entry entry = (Entry)it.next();
      entry.postDate();
    }
    transactionBundle.getListManager().add(entriesToAdd);
  }
//...
}

Implementing the sprout method

  1. Identify

  2. Add the reference in the code

  3. Inputs and outputs as arguments

  4. Identify return values

  5. Implementation of the method via TDD

  6. Delete comment and enable the call.

Advantages of the sprout method

  • Clear separation between the old code and the new one.

  • The affected variables can be identified.

  • The new method includes tests.

Disadvantages of the sprout method

  • You can leave the original method in a weird state (invocations to external functions).

  • You can leave the code in limbo (The new method may not be associated with the responsibility of the original code)

Sprout class

std::string QuarterlyReportGenerator::generate() {
  std::vector<Result> results = database.queryResults(
  beginDate, endDate);
  std::string pageText;
  pageText += "<html><head><title>"
    "Quarterly Report"
    "</title></head><body><table>";
  if (results.size() != 0) {
    for (std::vector<Result>::iterator it = results.begin();it != results.end();++it) {
      pageText += "<tr>";
      pageText += "<td>" + it->department + "</td>";
      pageText += "<td>" + it->manager + "</td>";
      char buffer [128];
      sprintf(buffer, "<td>$%d</td>", it->netProfit / 100);
      pageText += std::string(buffer);
      sprintf(buffer, "<td>$%d</td>", it->operatingExpense / 100);
      pageText += std::string(buffer);
      pageText += "</tr>";
    }
  } else {
    pageText += "No results for this period";
  }
  pageText += "</table>";
  pageText += "</body>";
  pageText += "</html>";
  return pageText;
}

Add feature

Add a header to the type list

<tr><td>Department</td><td>Manager</td><td>Profit</td><td>Expenses</td></tr>

Extracting logic to class

using namespace std;
class QuarterlyReportTableHeaderProducer{
  public:
    string makeHeader();
};

string QuarterlyReportTableProducer::makeHeader(){
  return "<tr><td>Department</td><td>Manager</td>"
  "<td>Profit</td><td>Expenses</td>";
}

We add invocation to the original method

QuarterlyReportGenerator::generate():
//...
QuarterlyReportTableHeaderProducer producer;
pageText += producer.makeHeader();
//...

Are you serious? A method for this

Yes

Allow additional refactoring

An interface

class QuarterlyReportTableHeaderGenerator{
  public:
  string generate();
};

Different implementations

class HTMLGenerator{
  public:
    virtual ~HTMLGenerator() = 0;
    virtual string generate() = 0;
};

class QuarterlyReportTableHeaderGenerator : public HTMLGenerator {
  public:
  //...
    virtual string generate();
  //...
};
class QuarterlyReportGenerator : public HTMLGenerator {
  public:
  //...
    virtual string generate();
  //...
};

Another example, filtering lists

Public Class ManagerProdFin
{
    Public Sub BuscarAltas(ByVal fechadesde As Date, ByVal fechahasta As Date, ByVal solovigentes As Boolean)
    {
        '...
        Dim listaapoderadosaltas = From apo As Apoderados In ListaApoderados Where
        Convert.ToInt32(apo.aponrocuenta) = Convert.ToInt32(obj.NroCliente)
        AndAlso Not apo.apotipodocumento.Equals("UBO")
        AndAlso (Not solovigentes OrElse
        Not apo.apoFechaVencimiento.HasValue()
        OrElse apo.apoFechaVencimiento.Value.CompareTo(fechahasta) > 0) Select apo
        '...
    }
}

New version

Public Class ManagerProdFin
{
    Public Sub BuscarAltas(ByVal fechadesde As Date, ByVal fechahasta As Date, ByVal solovigentes As Boolean)
    {
    '...
        Dim listatitularesaltas As List(Of Titulares) = New List(Of Titulares)
        listatitularesaltas = AuxManager.titularesvinculados(obj, ListaTitulares)
    '...
    }
}

The Sprout class

Public Class ManagerAuxiliar
    Public Sub New()
    End Sub
    Public Function titularesvinculados(ByVal ObjCustomer As Customer_Account,
 ByVal archivotitulares As List(Of Titulares)) As List(Of Titulares)
        Dim archivofiltrado = New List(Of Titulares)
        Dim listafiltrada = From titu As Titulares In archivotitulares
        Where Convert.ToInt32(titu.titubase_no) = ObjCustomer.NroClienteAsInt() Select titu
        For Each T In listafiltrada
            archivofiltrado.Add(T)
        Next
        Return archivofiltrado
    End Function
End Class

And the test

Imports NUnit.Framework

<TestFixture()>
Public Class ManagerAuxiliarUnitTest
    <Test()>
    Public Sub GivenObjCustandtitulares_WhenTvinculados_ThenArchfiltrado()
        Dim AuxiliaryM As ManagerAuxiliar = New ManagerAuxiliar()
        Dim titularesSet As List(Of Titulares) = New List(Of Titulares)
        titularesSet.Add(New Titulares With {.titubase_no = "0000123"})
        titularesSet.Add(New Titulares With {.titubase_no = "0000124"})
        titularesSet.Add(New Titulares With {.titubase_no = "0000125"})
        Dim Customer As Customer_Account = New Customer_Account()
        Customer.campocustacc = "000124"

        Dim result As List(Of Titulares) = AuxiliaryM.titularesvinculados(Customer, titularesSet)

        Assert.That(result, Has.Count().EqualTo(1))
    End Sub
End Class

Implementing the sprout class

  1. Identify where you need to change your code.

  2. Can it be grouped in a method? Think of a class name and invoke the method. Comment on the invocation.

  3. Variables and arguments.

  4. Return values.

  5. Develop the sprout class using TDD.

  6. Uncomment and invoke.

Advantages of the sprout class

  • Allows you to advance in development with more confidence than if you make invasive changes.

Disadvantages of the sprout class

  • Conceptual complexity.

Wrap Method

public class Employee{
//...
  public void pay() {
    Money amount = new Money();
    for (Iterator it = timecards.iterator(); it.hasNext(); ) {
      Timecard card = (Timecard)it.next();
      if (payPeriod.contains(date)) {
        amount.add(card.getHours() * payRate);
      }
    }
    payDispatcher.pay(this, date, amount);
  }
}

Add feature

Update a file every time a payment is run to employees

We can add the functionality at the end of the method, but what if …​

public class Employee {
  private void dispatchPayment() { (1)
    Money amount = new Money();
    for (Iterator it = timecards.iterator(); it.hasNext(); ) {
      Timecard card = (Timecard)it.next();
      if (payPeriod.contains(date)) {
        amount.add(card.getHours() * payRate);
      }
    }
    payDispatcher.pay(this, date, amount);
  }
  public void pay() { (2)
    logPayment(); (3)
    dispatchPayment(); (4)
  }
  private void logPayment() {
    //...
  }
}

Another alternative is to keep both functionalities

public class Employee {
  public void makeLoggedPayment() {
    logPayment();
    pay();
  }
  public void pay() {
  //...
  }
  private void logPayment() {
  //...
  }
}

Implementing the wrap method (Option 1)

  1. Identify the method you need to change.

  2. If the change is to be made as a separate method, rename the method and create a new method with the same name and signature as the previous method.

    • It is important to keep the signature **.

  3. Invoke the old method from the new method.

  4. Develop the new functionality using tests, then call from within the new method.

Implementing the wrap method (Option 2)

  1. Identify the method you need to change.

  2. If the change can be formulated as a single sequence of statements in one place, develop a new method using tests.

  3. Create another method that calls the new method and the old method.

Advantages of the wrap method

  • Unlike the sprout methods, the original methods do not change. With the sprout technique, at least one line is added.

  • The new functionality is independent of the existing functionality.

Disadvantages of the wrap method

  • Difficulty in naming the new methods.

There are only two hard things in Computer Science: cache invalidation and naming things. - Phil Karlton.

Wrap Class / Decorator pattern

class LoggingEmployee extends Employee {
  public LoggingEmployee(Employee e) {
    employee = e;
  }
  public void pay() {
    logPayment();
    employee.pay();
  }
  private void logPayment() {
    //...
  }
  //...
}

Wrap Class / Without decorator pattern

class LoggingPayDispatcher {
  private Employee e;
  public LoggingPayDispatcher(Employee e) {
    this.e = e;
  }
  public void pay() {
    employee.pay();
    logPayment();
  }
  private void logPayment() {
    //...
  }
  //...
}

Implementing the Wrap Class (Option 1) 1/2

  1. Identify a method where you need to make a change.

  2. If the change as a separate method, create a class that accepts the class to be modified as a constructor argument.

  3. Create a method in that class, using tests, and implement the new functionality. Write another method that calls the new method and the old method in the Wrap class.

  4. Create an instance of the Wrap class in the place where the new functionality needs to be enabled.

Tips for using the Wrap Class

  1. The behavior I want to add is completely independent, and I don’t want to pollute the existing class with low-level or unrelated behavior.

  2. The class has grown so much that I really can’t bear to make it worse. In a case like this, the wrap is created only as a base for later changes.

Are there more techniques?

Yes

Catalog of refactorings: https://refactoring.com/catalog/

Conclusions

  • Working with legacy code is difficult

  • Tests are necessary

  • When the disease cannot be prevented, it has to be treated

  • The application of these techniques depends on the context

  • The idea of these techniques is to support them with unit tests, without applying test only add more legacy code

Juan Moreno

I help developers to deliver good quality software following the best practices and applying clean code principles - Christian - Husband