Chapter 6: I Don't Have Much Time, and I Have to Change It (1)

Chapter 6: I Don't Have Much Time, and I Have to Change It (1)

Note of Working Effectively with Legacy Code

I can feel that when the author says someone doesn't have much time because that person is me. As a developer, and in a not offensive way, as a developer from China, I often don't have much time because there will always be some new requirement for me to implement.

That is not our developer's problem; the lousy management causes it, but we are the person who takes the consequences eventually, and that's unfair.

The author gives us so-called ninja techniques to solve these problems, which means you shouldn’t use them frequently cause ninjas won't show themselves that much in Japan. By the way, there is barely any ninja who will live long.

Let’s move on.

Sprout Method

An Example

Ok, let’s cut the bullshit and look through this code snippet.

class Entry {
    constructor(private num: number) {
    }
    public postDate(): void {
        console.log(this.num + '-post method was called');
    }
}
class TransactionGate {
    public postEntries(entries: Array<Entry>): void {
        for (let i = 0; i < entries.length; i++) {
            entries[i].postDate();
        }
        this.doSomethingElse(entries);
    }

    public doSomethingElse(entries: Array<Entry>): void {
        console.log('got entries');
        console.log(entries);
    }
}

const transactionGate = new TransactionGate();
const entries = new Array<Entry>();
entries.push(new Entry(1));
entries.push(new Entry(2));
entries.push(new Entry(3));
transactionGate.postEntries(entries);

I am using the style the author shows in the book. There are some better ways to implement the same feature, and I will show you later. After all, I am a frontend developer writing the Typescript rather than the Java code shown in the book.

Here comes a new requirement: Our boss wants us to implement a feature that can verify that none of the new entries are already in the transaction bundle before we post their dates and add them.

I believe that most of you will implement it in this way:

class TransactionGate {
    public postEntries(entries: Array<Entry>): void {
        // introduced a new variable, but it’s fine.
        const result = new Array<Entry>();
        for (let i = 0; i < entries.length; i++) {
            const entry = entries[i];
            // here comes the difference.
            if (this.checkTransactionBundleHas(entry)) {
                result.push(entry);
                entries[i].postDate();
            }
        }
        this.doSomethingElse(result);
    }

    public checkTransactionBundleHas(entry: Entry): boolean {
        // yeah, I know it is ugly, but the purpose here is understanding the concept.
        return entry.getNum() === 3;
    }

    public doSomethingElse(entries: Array<Entry>): void {
        console.log('got entries');
        console.log(entries);
    }
}

So what's the problem with this way of changing? As the author said, we're mingling two operations, and that is not good because you can barely expand your code when such operations turn into a debacle.

As the author told us, we should separate the operations into two methods.

class TransactionGate {
    // did you get it?
    public uniqueEntries(entries: Array<Entry>): Array<Entry> {
        const newEntries = new Array<Entry>();
        for (let i = 0; i < entries.length; i++) {
            const entry = entries[i];
            if (!this.checkTransactionBundleHas(entry)) {
                newEntries.push(entry);
            }
        }
        return newEntries;
    }
    public postEntries(entries: Array<Entry>): void {
        const result = this.uniqueEntries(entries);
        for (let i = 0; i < result.length; i++) {
            result[i].postDate();
        }
        this.doSomethingElse(result);
    }
    // irrelevant code was omitted
}

That is better. This way, we can expand our code quickly; all you need to do is write a new method and ensure that each approach focuses on solving only one problem.

And that is the basic concept of Functional Programming. I will implement the same feature in that way; let’s see this:

class TransactionGateFP {
    public postEntries(entries: Array<Entry>): void {
        const result = entries.filter(
          entry => !this.checkTransactionBundleHas(entry));
        result.forEach(entry => entry.postDate());
        this.doSomethingElse(result);
    }

    // the unique entries method is deleted, and the others remain the same.
}

See, It's more elegant.

But let's go back to the author's approach. He introduced a new method to handle the new logic, so the Methods sprouted.

Then the author talked a little about static methods; I think the filter method is equal to the static method cause we can call it almost everywhere as long as the instance is iterable, no matter what we want the single of it to do, in this case, we want to check if the TransactionBundle has the entry. It will only care if the result is True or not and do the filter.

Advantages and Disadvantages

From my point of view, there is no disadvantage. It's a basic approach that can let us deal with the new requirements without changing the existing source code. I did it sometimes; it is okay if we add the unit test for the current source code later. Otherwise, we are creating the ninja method.

But that is precisely its advantage. I mean the ninja method. What if we can't make the source code under unit tests? At least this approach can let us have some solid code under unit tests.

Writing unit tests for the new method is simple; you only need to instantiate the class and call that method.

Sprout Class

So here comes the trouble. What if we can't simply instantiate the class to which we will add our method? I mean, sometimes it happens because our class may have lots of dependencies that we can't solve in a short time.

Here we introduce the Sprout Class; it makes sense if we can't solve problems at the Method level, we turn to the Class level.

When we talk about Class, it is very natural for us to introduce Interface. We should extract similar concepts to the Interface; this movement will make our code more readable and extendable.

Advantages and Disadvantages

As for readable, there could be some little controversy. The logic we separated may fall apart if the source code is complex enough cause we hid the details during the separation; sometimes, that is the price of writing solid code based on the existing source code. Don't worry; we can always afford it.

Wrap Method

Two forms of Wrap Method

There are two forms of the Wrap Method. It depends on the way we deal with the existing source code.

The first one is to let the existing concept be untouched; when I say Concept, I often mean the Method of the Interface. Let's see the code.

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); 
    }
}

Now here is a new requirement, we want to log this payment. Besides, we want the pay Method remains its function and name.

public class Employee {
  private void dispatchPayment() {
    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() { 
    logPayment();
    dispatchPayment();
  }
  private void logPayment() { 
    ...
  }
}

We encapsulate the original pay Method as the DispatchPayment Method and put it into the new pay Method. But who calls this method won't feel the difference.

The author seems prefer the second one:

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

Yeah, It indeed is better. But what the hell is "makeLoggedPayment"?

So the author improves it like this:

public class Employee {
  public void calculatePay() { 
    ...
  }
  public void pay() { 
    logPayment();
    Money amount = calculatePay();
    dispatchPayment(amount);
  }
  private void logPayment() { 
    ...
  }
}

That is way better;

Wrap Class

See the next chapter please.

Summary

See the next chapter please.