Skip to content
wannacode

Exercise: a messy class, cleaned up step by step

In short: A refactoring exercise: a scooter rental class is cleaned up in five small steps (names, magic numbers, money, query apart from command, plans) while the tests stay green.

Problem

The scooters nobody dares to touch

A company that rents out e-scooters bills every ride with the RideMgr class. It has worked for three years. The calc method works out the price from the customer's plan (pay as you go, student or monthly pass), drops the unlock fee from the fourth ride of the day, adds a night surcharge, adds up the revenue and sends a receipt:

export class RideMgr {
  constructor() {
    this.c = new Map();
    this.s = 0;
    this.out = [];
  }


  // Price: €1 to unlock and €0.15 per minute.
  // At night the price goes up by 50%.
  calc(to, t, m, f) {
    this.c.set(to, (this.c.get(to) ?? 0) + 1);
    let p = 0;
    if (t === "payg") {
      p = 1 + m * 0.2;
    } else if (t === "student") {
      p = (1 + m * 0.2) * 0.8;
    } else if (t === "monthly") {
      if (m > 45) {
        p = (m - 45) * 0.2;
      }
    }
    if (this.c.get(to) > 3 && t !== "monthly") {
      p = p - 1;
    }
    if (f) {
      p = p + 0.5;
    }
    this.s = this.s + p;
    this.out.push(`${to}: ${m} min, €${p.toFixed(2)}`);
    return p;
  }
}

Next month two changes arrive:

  • a business plan: no unlock fee, €0.18 per minute, no discount for the fourth ride;
  • the price before the end: the app shows what the ride will cost while it is still going.

The second one cannot be done with this class. Here is what happens if the app calls calc to show the price, and then again to take the money:

const rides = new RideMgr();
const shown = rides.calc("ana@example.com", "payg", 10, false);
console.log(`Price before the end: €${shown.toFixed(2)}`);
const charged = rides.calc("ana@example.com", "payg", 10, false);
console.log(`Charged: €${charged.toFixed(2)}`);
console.log(`Receipts: ${rides.out.length}`);
console.log(`Revenue: €${rides.s.toFixed(2)}`);
Output
Price before the end: €3.00
Charged: €3.00
Receipts: 2
Revenue: €6.00

One ride, two receipts and €6 of revenue. calc sounds like a question, but it counts, adds up and sends.

There is no new idea in this lesson. It is an exercise: the class goes through the tools of the module one by one, in small steps, and you run the tests after each step.

See it

Step 0: the safety net

First the net, as in the lesson on tests. Nine tests pin what the class does today: the price for each plan, the night, the fourth ride, the receipt and the revenue.

import { test, beforeEach } from "node:test";
import assert from "node:assert/strict";
import { RideMgr } from "./rides.js";


const EMAIL = "ana@example.com";
let rides;


beforeEach(() => {
  rides = new RideMgr();
});


function ride(plan, minutes, night) {
  return money(rides.calc(EMAIL, plan, minutes, night));
}


function money(amount) {
  return amount.toFixed(2);
}


test("pay as you go", () => {
  assert.equal(ride("payg", 10, false), "3.00");
});


test("student", () => {
  assert.equal(ride("student", 10, false), "2.40");
});


test("monthly pass up to 45 min", () => {
  assert.equal(ride("monthly", 30, false), "0.00");
});


test("monthly pass over 45 min", () => {
  assert.equal(ride("monthly", 60, false), "3.00");
});


test("at night", () => {
  assert.equal(ride("payg", 10, true), "3.50");
});


test("fourth ride without unlock fee", () => {
  ride("payg", 10, false);
  ride("payg", 10, false);
  ride("payg", 10, false);
  assert.equal(ride("payg", 10, false), "2.00");
});


test("student on the fourth ride (as it is today)", () => {
  // As it is today: the discount is the full €1, not €0.80.
  ride("student", 10, false);
  ride("student", 10, false);
  ride("student", 10, false);
  assert.equal(ride("student", 10, false), "1.40");
});


test("the receipt", () => {
  ride("payg", 10, false);
  assert.deepEqual(rides.out, ["ana@example.com: 10 min, €3.00"]);
});


test("revenue", () => {
  ride("payg", 10, false);
  ride("student", 10, false);
  assert.equal(money(rides.s), "5.40");
});
Result
PASS  at night
PASS  fourth ride without unlock fee
PASS  monthly pass over 45 min
PASS  monthly pass up to 45 min
PASS  pay as you go
PASS  revenue
PASS  student
PASS  student on the fourth ride (as it is today)
PASS  the receipt
Tests: 9, passed: 9, failed: 0

The tests call the class only through the ride helper. When the shape of the class changes in the next steps, the helper and the calls will change, and the expected values never.

One of the tests is odd. A student pays 80% of the price, so their unlock costs €0.80. On the fourth ride, though, the class takes off the full €1, and the student pays €1.40 instead of €1.60. Is it a bug or a gift? You do not decide, and you do not decide now. The test pins what happens today, with a comment, and the question goes to the product owner. While you clean up, the behavior does not change, even when it looks wrong.

More precisely, if you are curious

The amounts are compared as text with two decimals ("2.40"), the way the customer sees them. Comparing numbers would trip over the small binary errors of number:

console.log((1 + 10 * 0.2) * 0.8);
Output
2.4000000000000004

And in step 3 the type of the amounts will change. The text with two decimals is the same before and after.

See it

Step 1: the names and the flag

t, m, f, p, c and s become words (names). The most important name is the method's: it does not calculate, it finishes the ride. The honest name is finishRide, and now the side effect shows in the name. The class becomes RideBilling.

export class RideBilling {
  constructor() {
    this.ridesToday = new Map();
    this.revenue = 0;
    this.receipts = [];
  }


  // Price: €1 to unlock and €0.15 per minute.
  // At night the price goes up by 50%.
  finishRide(email, plan, minutes, { night = false } = {}) {
    this.ridesToday.set(email, (this.ridesToday.get(email) ?? 0) + 1);
    let price = 0;
    if (plan === "payg") {
      price = 1 + minutes * 0.2;
    } else if (plan === "student") {
      price = (1 + minutes * 0.2) * 0.8;
    } else if (plan === "monthly") {
      if (minutes > 45) {
        price = (minutes - 45) * 0.2;
      }
    }
    if (this.ridesToday.get(email) > 3 && plan !== "monthly") {
      price = price - 1;
    }
    if (night) {
      price = price + 0.5;
    }
    this.revenue = this.revenue + price;
    this.receipts.push(`${email}: ${minutes} min, €${price.toFixed(2)}`);
    return price;
  }
}

The flag is left (functions). true in the call says nothing. Night is data about the ride, not a choice of a different job, so it does not become two functions. The call has to say what it means:

In JavaScript night comes in an options object, { night: true }, and day goes without it.
Before
function ride(plan, minutes, night) {
  return money(rides.calc(EMAIL, plan, minutes, night));
}


test("at night", () => {
  assert.equal(ride("payg", 10, true), "3.50");
});
After
function ride(plan, minutes, options) {
  return money(billing.finishRide(EMAIL, plan, minutes, options));
}


test("at night", () => {
  assert.equal(ride("payg", 10, { night: true }), "3.50");
});
The tests
PASS  at night
PASS  fourth ride without unlock fee
PASS  monthly pass over 45 min
PASS  monthly pass up to 45 min
PASS  pay as you go
PASS  revenue
PASS  student
PASS  student on the fourth ride (as it is today)
PASS  the receipt
Tests: 9, passed: 9, failed: 0
In VS Code, rename is F2. For the object's fields (this.s) the editor does not always know which object is which, so here the tests tell you more than in Java or C#.

See it

Step 2: the numbers and the comment that lies

1.0, 0.2, 0.8, 45, 3 and 0.5 become constants with names (magic number → constant).

The comment above the method says €0.15 per minute and a 50% night surcharge. The code charges €0.20 and adds €0.50. Which is true? The tests pinned what the code does, and customers have paid by it for years. The comment lies (comments) and is deleted. The constants say the same thing and cannot go stale on their own.

const UNLOCK_FEE = 1;
const PER_MINUTE = 0.2;
const STUDENT_SHARE = 0.8;
const MONTHLY_FREE_MINUTES = 45;
const PAID_UNLOCKS_PER_DAY = 3;
const NIGHT_SURCHARGE = 0.5;


export class RideBilling {
  constructor() {
    this.ridesToday = new Map();
    this.revenue = 0;
    this.receipts = [];
  }


  finishRide(email, plan, minutes, { night = false } = {}) {
    this.ridesToday.set(email, (this.ridesToday.get(email) ?? 0) + 1);
    let price = 0;
    if (plan === "payg") {
      price = UNLOCK_FEE + minutes * PER_MINUTE;
    } else if (plan === "student") {
      price = (UNLOCK_FEE + minutes * PER_MINUTE) * STUDENT_SHARE;
    } else if (plan === "monthly") {
      if (minutes > MONTHLY_FREE_MINUTES) {
        price = (minutes - MONTHLY_FREE_MINUTES) * PER_MINUTE;
      }
    }
    if (this.ridesToday.get(email) > PAID_UNLOCKS_PER_DAY && plan !== "monthly") {
      price = price - UNLOCK_FEE;
    }
    if (night) {
      price = price + NIGHT_SURCHARGE;
    }
    this.revenue = this.revenue + price;
    this.receipts.push(`${email}: ${minutes} min, €${price.toFixed(2)}`);
    return price;
  }
}
The tests
PASS  at night
PASS  fourth ride without unlock fee
PASS  monthly pass over 45 min
PASS  monthly pass up to 45 min
PASS  pay as you go
PASS  revenue
PASS  student
PASS  student on the fourth ride (as it is today)
PASS  the receipt
Tests: 9, passed: 9, failed: 0

The tests are not touched: neither the name nor the calls have changed.

The constants are at module level, above the class: that way any other code in the file sees them, and in step 5 so do the plans.

Try it

Find the bug

A colleague does the same step, but trusts the comment and keeps it:

Pick the line with the bug.

More precisely, if you are curious
The tests
FAIL  at night
FAIL  fourth ride without unlock fee
FAIL  monthly pass over 45 min
FAIL  pay as you go
FAIL  revenue
FAIL  student
FAIL  student on the fourth ride (as it is today)
FAIL  the receipt
PASS  monthly pass up to 45 min
Tests: 9, passed: 1, failed: 8

See it

Step 3: the money

Money in a fractional number is primitive obsession (smells). Today it gives no wrong cent in any test, because every amount is rounded only when it is printed. That is why the step is a refactoring: the output is the same. But the revenue is a sum of thousands of such amounts, and that is where the small errors pile up.

JavaScript has no decimal type, so the money becomes whole cents. The names say the unit (_CENTS), the student price is 80 percent of the cents, rounded with Math.round, and the euros function turns cents into euros only when printing.
const UNLOCK_FEE_CENTS = 100;
const PER_MINUTE_CENTS = 20;
const STUDENT_PERCENT = 80;
const MONTHLY_FREE_MINUTES = 45;
const PAID_UNLOCKS_PER_DAY = 3;
const NIGHT_SURCHARGE_CENTS = 50;


function euros(cents) {
  return (cents / 100).toFixed(2);
}


export class RideBilling {
  constructor() {
    this.ridesToday = new Map();
    this.revenue = 0;
    this.receipts = [];
  }


  finishRide(email, plan, minutes, { night = false } = {}) {
    this.ridesToday.set(email, (this.ridesToday.get(email) ?? 0) + 1);
    let price = 0;
    if (plan === "payg") {
      price = UNLOCK_FEE_CENTS + minutes * PER_MINUTE_CENTS;
    } else if (plan === "student") {
      price = Math.round(((UNLOCK_FEE_CENTS + minutes * PER_MINUTE_CENTS) * STUDENT_PERCENT) / 100);
    } else if (plan === "monthly") {
      if (minutes > MONTHLY_FREE_MINUTES) {
        price = (minutes - MONTHLY_FREE_MINUTES) * PER_MINUTE_CENTS;
      }
    }
    if (this.ridesToday.get(email) > PAID_UNLOCKS_PER_DAY && plan !== "monthly") {
      price = price - UNLOCK_FEE_CENTS;
    }
    if (night) {
      price = price + NIGHT_SURCHARGE_CENTS;
    }
    this.revenue = this.revenue + price;
    this.receipts.push(`${email}: ${minutes} min, €${euros(price)}`);
    return price;
  }
}
In the tests only the helper that turns the amount into text changes:
function money(cents) {
  return (cents / 100).toFixed(2);
}
The tests
PASS  at night
PASS  fourth ride without unlock fee
PASS  monthly pass over 45 min
PASS  monthly pass up to 45 min
PASS  pay as you go
PASS  revenue
PASS  student
PASS  student on the fourth ride (as it is today)
PASS  the receipt
Tests: 9, passed: 9, failed: 0

Try it

Fill in

The price per minute in the language's type for money:

const PER_MINUTE_CENTS = ;

See it

Step 4: the question apart from the command

Now the change that was waiting: the price before the end has to be a question that changes nothing (command or question). The calculation is extracted into a price method (Extract Method from the refactoring lesson). finishRide stays a command: it asks for the price, then counts, adds up and sends.

A colleague does it like this. The counter moves after the price, because a ride counts once it has finished:

What do the nine tests say?

price(email, plan, minutes, { night = false } = {}) {
  let price = 0;
  if (plan === "payg") {
    price = UNLOCK_FEE_CENTS + minutes * PER_MINUTE_CENTS;
  } else if (plan === "student") {
    price = Math.round(((UNLOCK_FEE_CENTS + minutes * PER_MINUTE_CENTS) * STUDENT_PERCENT) / 100);
  } else if (plan === "monthly") {
    if (minutes > MONTHLY_FREE_MINUTES) {
      price = (minutes - MONTHLY_FREE_MINUTES) * PER_MINUTE_CENTS;
    }
  }
  if ((this.ridesToday.get(email) ?? 0) > PAID_UNLOCKS_PER_DAY && plan !== "monthly") {
    price = price - UNLOCK_FEE_CENTS;
  }
  if (night) {
    price = price + NIGHT_SURCHARGE_CENTS;
  }
  return price;
}


finishRide(email, plan, minutes, { night = false } = {}) {
  const price = this.price(email, plan, minutes, { night });
  this.ridesToday.set(email, (this.ridesToday.get(email) ?? 0) + 1);
  this.revenue = this.revenue + price;
  this.receipts.push(`${email}: ${minutes} min, €${euros(price)}`);
  return price;
}
Answer
More precisely, if you are curious
The tests
FAIL  fourth ride without unlock fee: 3.00 !== 2.00
FAIL  student on the fourth ride (as it is today): 2.40 !== 1.40
PASS  at night
PASS  monthly pass over 45 min
PASS  monthly pass up to 45 min
PASS  pay as you go
PASS  revenue
PASS  student
PASS  the receipt
Tests: 9, passed: 7, failed: 2

See it

Step 4, second try

The red points at the last step. You do not look for the bug in the whole class: you undo the step and do it again. The order of the side effects was a hidden part of the behavior, and only the tests knew about it.

price works out which ride of the day this will be:

price(email, plan, minutes, { night = false } = {}) {
  const rideOfDay = (this.ridesToday.get(email) ?? 0) + 1;
  let price = 0;
  if (plan === "payg") {
    price = UNLOCK_FEE_CENTS + minutes * PER_MINUTE_CENTS;
  } else if (plan === "student") {
    price = Math.round(((UNLOCK_FEE_CENTS + minutes * PER_MINUTE_CENTS) * STUDENT_PERCENT) / 100);
  } else if (plan === "monthly") {
    if (minutes > MONTHLY_FREE_MINUTES) {
      price = (minutes - MONTHLY_FREE_MINUTES) * PER_MINUTE_CENTS;
    }
  }
  if (rideOfDay > PAID_UNLOCKS_PER_DAY && plan !== "monthly") {
    price = price - UNLOCK_FEE_CENTS;
  }
  if (night) {
    price = price + NIGHT_SURCHARGE_CENTS;
  }
  return price;
}


finishRide(email, plan, minutes, { night = false } = {}) {
  const price = this.price(email, plan, minutes, { night });
  this.ridesToday.set(email, (this.ridesToday.get(email) ?? 0) + 1);
  this.revenue = this.revenue + price;
  this.receipts.push(`${email}: ${minutes} min, €${euros(price)}`);
  return price;
}
The tests
PASS  at night
PASS  fourth ride without unlock fee
PASS  monthly pass over 45 min
PASS  monthly pass up to 45 min
PASS  pay as you go
PASS  revenue
PASS  student
PASS  student on the fourth ride (as it is today)
PASS  the receipt
Tests: 9, passed: 9, failed: 0

The tests are not touched again: they call finishRide, and it does the same as before.

The whole class after step 4
const UNLOCK_FEE_CENTS = 100;
const PER_MINUTE_CENTS = 20;
const STUDENT_PERCENT = 80;
const MONTHLY_FREE_MINUTES = 45;
const PAID_UNLOCKS_PER_DAY = 3;
const NIGHT_SURCHARGE_CENTS = 50;


function euros(cents) {
  return (cents / 100).toFixed(2);
}


export class RideBilling {
  constructor() {
    this.ridesToday = new Map();
    this.revenue = 0;
    this.receipts = [];
  }


  price(email, plan, minutes, { night = false } = {}) {
    const rideOfDay = (this.ridesToday.get(email) ?? 0) + 1;
    let price = 0;
    if (plan === "payg") {
      price = UNLOCK_FEE_CENTS + minutes * PER_MINUTE_CENTS;
    } else if (plan === "student") {
      price = Math.round(((UNLOCK_FEE_CENTS + minutes * PER_MINUTE_CENTS) * STUDENT_PERCENT) / 100);
    } else if (plan === "monthly") {
      if (minutes > MONTHLY_FREE_MINUTES) {
        price = (minutes - MONTHLY_FREE_MINUTES) * PER_MINUTE_CENTS;
      }
    }
    if (rideOfDay > PAID_UNLOCKS_PER_DAY && plan !== "monthly") {
      price = price - UNLOCK_FEE_CENTS;
    }
    if (night) {
      price = price + NIGHT_SURCHARGE_CENTS;
    }
    return price;
  }


  finishRide(email, plan, minutes, { night = false } = {}) {
    const price = this.price(email, plan, minutes, { night });
    this.ridesToday.set(email, (this.ridesToday.get(email) ?? 0) + 1);
    this.revenue = this.revenue + price;
    this.receipts.push(`${email}: ${minutes} min, €${euros(price)}`);
    return price;
  }
}

See it

Step 5: the plans

What is left is the chain of if on the text of the plan (long if chain). Every new plan is a new branch, and "monthly" is checked in two places. Each plan becomes a class that knows its price and its discount for the fourth ride. This is Replace Conditional with Polymorphism from the refactoring lesson:

export class PayAsYouGo {
  price(minutes) {
    return UNLOCK_FEE_CENTS + minutes * PER_MINUTE_CENTS;
  }


  loyaltyDiscount() {
    return UNLOCK_FEE_CENTS;
  }
}


export class Student {
  price(minutes) {
    return Math.round((new PayAsYouGo().price(minutes) * STUDENT_PERCENT) / 100);
  }


  loyaltyDiscount() {
    // As it is today: the full €1, even though a student pays €0.80 to unlock.
    return UNLOCK_FEE_CENTS;
  }
}


export class Monthly {
  price(minutes) {
    return Math.max(0, minutes - MONTHLY_FREE_MINUTES) * PER_MINUTE_CENTS;
  }


  loyaltyDiscount() {
    return 0;
  }
}

The check for the monthly pass disappears: it simply returns a discount of 0. The student's odd behavior now sits on one line, with a comment that says why it is so (the good comment from the lesson on comments).

const UNLOCK_FEE_CENTS = 100;
const PER_MINUTE_CENTS = 20;
const STUDENT_PERCENT = 80;
const MONTHLY_FREE_MINUTES = 45;
const PAID_UNLOCKS_PER_DAY = 3;
const NIGHT_SURCHARGE_CENTS = 50;


function euros(cents) {
  return (cents / 100).toFixed(2);
}


export class RideBilling {
  constructor() {
    this.ridesToday = new Map();
    this.revenue = 0;
    this.receipts = [];
  }


  price(email, plan, minutes, { night = false } = {}) {
    const rideOfDay = (this.ridesToday.get(email) ?? 0) + 1;
    let price = plan.price(minutes);
    if (rideOfDay > PAID_UNLOCKS_PER_DAY) {
      price = price - plan.loyaltyDiscount();
    }
    if (night) {
      price = price + NIGHT_SURCHARGE_CENTS;
    }
    return price;
  }


  finishRide(email, plan, minutes, { night = false } = {}) {
    const price = this.price(email, plan, minutes, { night });
    this.ridesToday.set(email, (this.ridesToday.get(email) ?? 0) + 1);
    this.revenue = this.revenue + price;
    this.receipts.push(`${email}: ${minutes} min, €${euros(price)}`);
    return price;
  }
}
There is no interface here: each plan simply has the price and loyaltyDiscount methods, and the billing class calls them (duck typing). The constants stay at module level.

In the tests the calls pass an object instead of text:

assert.equal(ride("payg", 10, { night: true }), "3.50");
assert.equal(ride(new PayAsYouGo(), 10, { night: true }), "3.50");
The tests
PASS  at night
PASS  fourth ride without unlock fee
PASS  monthly pass over 45 min
PASS  monthly pass up to 45 min
PASS  pay as you go
PASS  revenue
PASS  student
PASS  student on the fourth ride (as it is today)
PASS  the receipt
Tests: 9, passed: 9, failed: 0

This is Strategy from the patterns module: the rule is a separate object, and billing only asks.

See it

The two changes now

The business plan is a new class. The billing class is not touched (open for extension):

const BUSINESS_PER_MINUTE_CENTS = 18;


export class Business {
  price(minutes) {
    return minutes * BUSINESS_PER_MINUTE_CENTS;
  }


  loyaltyDiscount() {
    return 0;
  }
}

The price before the end is the price question. However many times you call it, nothing is counted and no receipt goes out:

const billing = new RideBilling();
const payg = new PayAsYouGo();
billing.finishRide("ana@example.com", payg, 10);
billing.finishRide("ana@example.com", payg, 5);
billing.finishRide("ana@example.com", payg, 12, { night: true });
console.log(`Price before the end: €${(billing.price("ana@example.com", payg, 8) / 100).toFixed(2)}`);
console.log(`Price before the end: €${(billing.price("ana@example.com", payg, 8) / 100).toFixed(2)}`);
console.log(`Charged: €${(billing.finishRide("ana@example.com", payg, 8) / 100).toFixed(2)}`);
console.log(`Company, 12 min: €${(billing.finishRide("office@example.com", new Business(), 12) / 100).toFixed(2)}`);
console.log(`Receipts: ${billing.receipts.length}`);
Output
Price before the end: €1.60
Price before the end: €1.60
Charged: €1.60
Company, 12 min: €2.16
Receipts: 5

Five receipts: three rides before the preview, the fourth one and the company's. The two previews leave no trace, and they show exactly what is charged afterwards.

The idea

Before and after

Smell Step Lesson
nothing guards the behavior 0: tests that pin today's behavior the safety net
one-letter names, calc that does not say it sends 1: Rename names
true in the call 1: an argument that says what it means functions
magic numbers 2: constants with names names
a comment that lies 2: deleted comments
money in a fractional number 3: a type for money smells
a question with a hidden side effect, a long method 4: Extract Method, command and question functions, refactoring
a chain of if on the plan as text 5: Replace Conditional with Polymorphism smells, refactoring, SOLID

The class got longer, not shorter. But every rule is in one place and has a name, and each change from the start is one class.

The day of rides, run with the class from before step 1 and with the class after step 5, prints the same thing line by line. Here both programs run on every check of the site.

Before
const rides = new RideMgr();
rides.calc("ana@example.com", "payg", 10, false);
rides.calc("ivan@example.com", "student", 25, false);
rides.calc("ana@example.com", "payg", 5, false);
rides.calc("maria@example.com", "monthly", 60, true);
rides.calc("ana@example.com", "payg", 12, true);
rides.calc("ana@example.com", "payg", 8, false);


for (const receipt of rides.out) {
  console.log(receipt);
}
console.log(`Revenue: €${rides.s.toFixed(2)}`);
After
const billing = new RideBilling();
const payg = new PayAsYouGo();
billing.finishRide("ana@example.com", payg, 10);
billing.finishRide("ivan@example.com", new Student(), 25);
billing.finishRide("ana@example.com", payg, 5);
billing.finishRide("maria@example.com", new Monthly(), 60, { night: true });
billing.finishRide("ana@example.com", payg, 12, { night: true });
billing.finishRide("ana@example.com", payg, 8);


for (const receipt of billing.receipts) {
  console.log(receipt);
}
console.log(`Revenue: €${(billing.revenue / 100).toFixed(2)}`);
Output
ana@example.com: 10 min, €3.00
ivan@example.com: 25 min, €4.80
ana@example.com: 5 min, €2.00
maria@example.com: 60 min, €3.50
ana@example.com: 12 min, €3.90
ana@example.com: 8 min, €1.60
Revenue: €18.80

The idea

Where to stop

You stop when the changes that are coming become easy. Here the two changes are one class and one line. That is why some steps are deliberately not done:

  • the receipt in its own class. The text of the receipt is a second reason for the class to change (one reason to change). Today it does not change. As soon as marketing starts editing it every month, it becomes a separate class (task 1);
  • sending behind an interface. The receipts are a list, not a real email. When the real email arrives, the test must not send it, and that is when the abstraction comes (depending on an abstraction);
  • the student's behavior. That is a change in behavior, not a cleanup. It is done separately, with its own test, when the product decides.

And the rule from the refactoring lesson: either you change the shape or the behavior, never both in one step.

Exercise

Carry on yourself

Solve the tasks in your editor, starting from the class after step 5. After each step, run the nine tests.

1. The receipt in its own class. Move the text of the receipt and the list of sent receipts into a ReceiptBook class. The billing class only tells it "send". The tests are not touched.

One possible solution
class ReceiptBook {
  constructor() {
    this.sent = [];
  }


  send(email, minutes, price) {
    this.sent.push(`${email}: ${minutes} min, €${euros(price)}`);
  }
}
const UNLOCK_FEE_CENTS = 100;
const PER_MINUTE_CENTS = 20;
const STUDENT_PERCENT = 80;
const MONTHLY_FREE_MINUTES = 45;
const PAID_UNLOCKS_PER_DAY = 3;
const NIGHT_SURCHARGE_CENTS = 50;


function euros(cents) {
  return (cents / 100).toFixed(2);
}


export class RideBilling {
  constructor() {
    this.ridesToday = new Map();
    this.revenue = 0;
    this.receiptBook = new ReceiptBook();
  }


  price(email, plan, minutes, { night = false } = {}) {
    const rideOfDay = (this.ridesToday.get(email) ?? 0) + 1;
    let price = plan.price(minutes);
    if (rideOfDay > PAID_UNLOCKS_PER_DAY) {
      price = price - plan.loyaltyDiscount();
    }
    if (night) {
      price = price + NIGHT_SURCHARGE_CENTS;
    }
    return price;
  }


  finishRide(email, plan, minutes, { night = false } = {}) {
    const price = this.price(email, plan, minutes, { night });
    this.ridesToday.set(email, (this.ridesToday.get(email) ?? 0) + 1);
    this.revenue = this.revenue + price;
    this.receiptBook.send(email, minutes, price);
    return price;
  }


  get receipts() {
    return this.receiptBook.sent;
  }
}
The tests
PASS  at night
PASS  fourth ride without unlock fee
PASS  monthly pass over 45 min
PASS  monthly pass up to 45 min
PASS  pay as you go
PASS  revenue
PASS  student
PASS  student on the fourth ride (as it is today)
PASS  the receipt
Tests: 9, passed: 9, failed: 0

The program for the day from the "Before and after" card prints the same output with this class.

2. The plan from text. The plan comes from the database as text ("payg"). Write the table that turns it into an object. What did the old class do with an unknown plan? Pin it with a test on the old class before you decide what the new one should do.

3. Night from the hour. The app no longer knows when it is night; it passes the start time. The rule "from 22:00 to 6:00" has to live in one place. Write the tests for 21:59, 22:00, 5:59 and 6:00 first.

4. The student. The product has decided: on the fourth ride a student gets €0.80, not €1. That is a change in behavior: first change the expected value in the test and see it red, then change one line in the plan.

Test

Check yourself

1. The test for a student on the fourth ride expects €1.40, and the "right" answer looks like €1.60. What do you do while you clean up the class?
2. The comment says €0.15 per minute, and the code charges €0.20. Which one do you trust?
3. Why do the tests call the class through one helper and not directly?
4. The first try at step 4 turned two tests red. What do you do?
5. After the cleanup, the business plan is one new class, and the billing class is not touched. Which principle is this?

Made by Ivo Gergov as part of Evo Tech's campaign for accessible programming education ·