Case Study 2: Refactoring Spaghetti Code

Before and after: a 200-line monolithic program decomposed into procedures. Shows the design process, naming conventions, and how to test individual procedures.

The Scenario

Rosa Martinelli has been freelancing as a graphic designer for three years. She wrote a Pascal program to generate monthly invoices for her clients. The program works — it reads client information, lists services performed, calculates totals with tax, and prints a formatted invoice. But it is a single monolithic block of code: 190 lines between begin and end., no procedures, no functions.

Last month, a client asked Rosa to change the tax rate. She found the tax calculation buried on line 127 but missed a second calculation on line 163 that used the old rate. The invoice was wrong. She lost half a day finding and fixing the problem.

This case study walks through Rosa's refactoring process: identifying logical blocks, extracting procedures, choosing parameter modes, and arriving at a clean, maintainable design.

The Original Code (Before)

Here is a simplified version of Rosa's monolithic invoice program. Read it through and notice how difficult it is to find any specific piece of logic:

program InvoiceGenerator;
{ Rosa's original monolithic invoice program — BEFORE refactoring }
uses SysUtils;

const
  MaxServices = 20;
  TaxRate = 0.08;  { 8% sales tax }

var
  ClientName, ClientAddress, ClientEmail: string;
  ServiceDesc: array[1..MaxServices] of string;
  ServiceHours: array[1..MaxServices] of Real;
  ServiceRate: array[1..MaxServices] of Real;
  ServiceCount: Integer;
  I: Integer;
  LineTotal, Subtotal, TaxAmount, GrandTotal: Real;
  InvoiceNumber: Integer;
  InvoiceDate: string;
  Choice: Char;
  TempDesc: string;
  TempHours, TempRate: Real;
  PaymentTerms: string;

begin
  { Read client info }
  WriteLn('=== Invoice Generator ===');
  WriteLn;
  Write('Client name: ');
  ReadLn(ClientName);
  Write('Client address: ');
  ReadLn(ClientAddress);
  Write('Client email: ');
  ReadLn(ClientEmail);
  Write('Invoice number: ');
  ReadLn(InvoiceNumber);
  InvoiceDate := DateToStr(Date);
  WriteLn;

  { Read services }
  ServiceCount := 0;
  repeat
    Write('Add a service? (Y/N): ');
    ReadLn(Choice);
    if (Choice = 'Y') or (Choice = 'y') then
    begin
      if ServiceCount >= MaxServices then
      begin
        WriteLn('Maximum services reached (', MaxServices, ').');
        Choice := 'N';
      end
      else
      begin
        Inc(ServiceCount);
        Write('  Description: ');
        ReadLn(ServiceDesc[ServiceCount]);
        Write('  Hours: ');
        ReadLn(ServiceHours[ServiceCount]);
        Write('  Hourly rate: $');
        ReadLn(ServiceRate[ServiceCount]);
      end;
    end;
  until (Choice = 'N') or (Choice = 'n');
  WriteLn;

  { Payment terms }
  Write('Payment terms (e.g., "Net 30"): ');
  ReadLn(PaymentTerms);
  WriteLn;

  { Calculate totals }
  Subtotal := 0;
  for I := 1 to ServiceCount do
  begin
    LineTotal := ServiceHours[I] * ServiceRate[I];
    Subtotal := Subtotal + LineTotal;
  end;
  TaxAmount := Subtotal * TaxRate;
  GrandTotal := Subtotal + TaxAmount;

  { Print invoice header }
  WriteLn('================================================================');
  WriteLn('                         INVOICE');
  WriteLn('================================================================');
  WriteLn('Rosa Martinelli Design Studio');
  WriteLn('123 Creative Avenue, Portland, OR 97201');
  WriteLn('rosa@martinellidesign.com');
  WriteLn('----------------------------------------------------------------');
  WriteLn('Invoice #: ', InvoiceNumber);
  WriteLn('Date:      ', InvoiceDate);
  WriteLn('----------------------------------------------------------------');
  WriteLn('Bill To:');
  WriteLn('  ', ClientName);
  WriteLn('  ', ClientAddress);
  WriteLn('  ', ClientEmail);
  WriteLn('----------------------------------------------------------------');

  { Print service lines }
  WriteLn('Description':30, 'Hours':8, 'Rate':10, 'Amount':12);
  WriteLn('----------------------------------------------------------------');
  for I := 1 to ServiceCount do
  begin
    LineTotal := ServiceHours[I] * ServiceRate[I];
    WriteLn(ServiceDesc[I]:30, ServiceHours[I]:8:1, ServiceRate[I]:10:2,
            LineTotal:12:2);
  end;

  { Print totals }
  WriteLn('----------------------------------------------------------------');
  WriteLn('Subtotal:':50, Subtotal:12:2);
  WriteLn(Format('Tax (%.0f%%):', [TaxRate * 100]):50, TaxAmount:12:2);
  WriteLn('TOTAL DUE:':50, GrandTotal:12:2);
  WriteLn('================================================================');
  WriteLn;
  WriteLn('Payment Terms: ', PaymentTerms);
  WriteLn('Thank you for your business!');
  WriteLn('================================================================');
end.

Identifying the Problems

Rosa lists the issues with this monolithic approach:

1. The tax rate bug. TaxRate is used in two places (lines calculating TaxAmount in the totals section, and in the display formatting). If she introduces a variable tax rate, she has to hunt through the entire program.

2. Duplicated computation. LineTotal := ServiceHours[I] * ServiceRate[I] appears twice — once in the calculation section and once in the printing section. If the formula changes, both must be updated.

3. No testability. She cannot test the subtotal calculation without running the entire program and typing in all the client data.

4. Difficult navigation. Finding "where is the invoice header printed?" requires scrolling through client input code, service input code, and calculation code.

5. Variable soup. Fifteen variables in a single var section. Some are used only in the input section, some only in the output section, but they all share the same scope.

The Refactoring Process

Step 1: Identify Logical Blocks

Rosa highlights sections of code with a consistent purpose:

Step 2: Design Procedure Signatures

For each block, Rosa decides: procedure or function? What parameters? What modes?

{ Block A — reads data, so needs var parameters for outputs }
procedure ReadClientInfo(var Name, Address, Email: string;
                         var InvNumber: Integer; var InvDate: string);

{ Block B — reads and stores services, modifying arrays and count }
procedure ReadServices(var Descriptions: ServiceDescArray;
                       var Hours, Rates: ServiceNumArray;
                       var Count: Integer);

{ Block C — simple enough to stay inline or be one line }
procedure ReadPaymentTerms(var Terms: string);

{ Block D — computes values, so functions are appropriate }
function CalculateLineTotal(const Hours, Rate: Real): Real;
function CalculateSubtotal(const Hours, Rates: ServiceNumArray;
                           Count: Integer): Real;
function CalculateTax(Subtotal: Real): Real;

{ Block E — display only, reads all inputs }
procedure PrintInvoiceHeader(const ClientName, ClientAddress,
                             ClientEmail: string;
                             InvNumber: Integer;
                             const InvDate: string);

{ Block F — display only }
procedure PrintServiceLines(const Descriptions: ServiceDescArray;
                            const Hours, Rates: ServiceNumArray;
                            Count: Integer);

{ Block G — display only }
procedure PrintTotals(Subtotal, TaxAmount, GrandTotal: Real;
                      const PaymentTerms: string);

Key design choices: - Blocks that read input use var parameters for the data they fill in. - Blocks that display use const parameters (read but do not modify). - Blocks that compute values are functions that return their result. - CalculateLineTotal is a function because the line total is computed in two places. Now it is computed in one place and called twice. Bug fixed before it happens again.

Step 3: Define Types

Rosa introduces named types for her arrays, making parameter lists cleaner:

const
  MaxServices = 20;
  TaxRate = 0.08;

type
  ServiceDescArray = array[1..MaxServices] of string;
  ServiceNumArray  = array[1..MaxServices] of Real;

Step 4: Write the Procedures

Here is the full refactored program:

program InvoiceGeneratorRefactored;
{ Rosa's invoice program — AFTER refactoring into procedures/functions }
uses SysUtils;

const
  MaxServices = 20;
  TaxRate = 0.08;

type
  ServiceDescArray = array[1..MaxServices] of string;
  ServiceNumArray  = array[1..MaxServices] of Real;

{ --- Input Procedures --- }

procedure ReadClientInfo(var Name, Address, Email: string;
                         var InvNumber: Integer; var InvDate: string);
begin
  WriteLn('=== Invoice Generator ===');
  WriteLn;
  Write('Client name: ');
  ReadLn(Name);
  Write('Client address: ');
  ReadLn(Address);
  Write('Client email: ');
  ReadLn(Email);
  Write('Invoice number: ');
  ReadLn(InvNumber);
  InvDate := DateToStr(Date);
  WriteLn;
end;

procedure ReadServices(var Descriptions: ServiceDescArray;
                       var Hours, Rates: ServiceNumArray;
                       var Count: Integer);
var
  Choice: Char;
begin
  Count := 0;
  repeat
    Write('Add a service? (Y/N): ');
    ReadLn(Choice);
    if (Choice = 'Y') or (Choice = 'y') then
    begin
      if Count >= MaxServices then
      begin
        WriteLn('Maximum services reached (', MaxServices, ').');
        Choice := 'N';
      end
      else
      begin
        Inc(Count);
        Write('  Description: ');
        ReadLn(Descriptions[Count]);
        Write('  Hours: ');
        ReadLn(Hours[Count]);
        Write('  Hourly rate: $');
        ReadLn(Rates[Count]);
      end;
    end;
  until (Choice = 'N') or (Choice = 'n');
  WriteLn;
end;

procedure ReadPaymentTerms(var Terms: string);
begin
  Write('Payment terms (e.g., "Net 30"): ');
  ReadLn(Terms);
  WriteLn;
end;

{ --- Calculation Functions --- }

function CalculateLineTotal(const Hours, Rate: Real): Real;
begin
  Result := Hours * Rate;
end;

function CalculateSubtotal(const Hours, Rates: ServiceNumArray;
                           Count: Integer): Real;
var
  I: Integer;
begin
  Result := 0;
  for I := 1 to Count do
    Result := Result + CalculateLineTotal(Hours[I], Rates[I]);
end;

function CalculateTax(Subtotal: Real): Real;
begin
  Result := Subtotal * TaxRate;
end;

{ --- Display Procedures --- }

procedure PrintInvoiceHeader(const ClientName, ClientAddress,
                             ClientEmail: string;
                             InvNumber: Integer;
                             const InvDate: string);
begin
  WriteLn('================================================================');
  WriteLn('                         INVOICE');
  WriteLn('================================================================');
  WriteLn('Rosa Martinelli Design Studio');
  WriteLn('123 Creative Avenue, Portland, OR 97201');
  WriteLn('rosa@martinellidesign.com');
  WriteLn('----------------------------------------------------------------');
  WriteLn('Invoice #: ', InvNumber);
  WriteLn('Date:      ', InvDate);
  WriteLn('----------------------------------------------------------------');
  WriteLn('Bill To:');
  WriteLn('  ', ClientName);
  WriteLn('  ', ClientAddress);
  WriteLn('  ', ClientEmail);
  WriteLn('----------------------------------------------------------------');
end;

procedure PrintServiceLines(const Descriptions: ServiceDescArray;
                            const Hours, Rates: ServiceNumArray;
                            Count: Integer);
var
  I: Integer;
  LineAmt: Real;
begin
  WriteLn('Description':30, 'Hours':8, 'Rate':10, 'Amount':12);
  WriteLn('----------------------------------------------------------------');
  for I := 1 to Count do
  begin
    LineAmt := CalculateLineTotal(Hours[I], Rates[I]);
    WriteLn(Descriptions[I]:30, Hours[I]:8:1, Rates[I]:10:2, LineAmt:12:2);
  end;
end;

procedure PrintTotals(Subtotal, TaxAmt, GrandTotal: Real;
                      const PayTerms: string);
begin
  WriteLn('----------------------------------------------------------------');
  WriteLn('Subtotal:':50, Subtotal:12:2);
  WriteLn(Format('Tax (%.0f%%):', [TaxRate * 100]):50, TaxAmt:12:2);
  WriteLn('TOTAL DUE:':50, GrandTotal:12:2);
  WriteLn('================================================================');
  WriteLn;
  WriteLn('Payment Terms: ', PayTerms);
  WriteLn('Thank you for your business!');
  WriteLn('================================================================');
end;

{ --- Main Program --- }

var
  ClientName, ClientAddress, ClientEmail: string;
  Descriptions: ServiceDescArray;
  Hours, Rates: ServiceNumArray;
  ServiceCount: Integer;
  InvoiceNumber: Integer;
  InvoiceDate: string;
  PaymentTerms: string;
  Subtotal, TaxAmount, GrandTotal: Real;

begin
  ReadClientInfo(ClientName, ClientAddress, ClientEmail,
                 InvoiceNumber, InvoiceDate);
  ReadServices(Descriptions, Hours, Rates, ServiceCount);
  ReadPaymentTerms(PaymentTerms);

  Subtotal   := CalculateSubtotal(Hours, Rates, ServiceCount);
  TaxAmount  := CalculateTax(Subtotal);
  GrandTotal := Subtotal + TaxAmount;

  PrintInvoiceHeader(ClientName, ClientAddress, ClientEmail,
                     InvoiceNumber, InvoiceDate);
  PrintServiceLines(Descriptions, Hours, Rates, ServiceCount);
  PrintTotals(Subtotal, TaxAmount, GrandTotal, PaymentTerms);
end.

Before vs. After: A Comparison

The Refactoring Checklist

Rosa developed a checklist she uses whenever she suspects a program needs decomposition:

1. Identify repeated code

Any block of code that appears more than once is a candidate for a procedure or function. In the original, the line-total calculation appeared twice.

2. Identify logical blocks

Group consecutive lines that serve a single purpose. If you can describe a block with a verb phrase ("read the client info," "print the totals"), it should probably be a procedure.

3. Choose procedure vs. function

• Does the block produce a single value? Make it a function.
• Does the block perform an action (I/O, modification)? Make it a procedure.

4. Choose parameter modes

• Will the caller's data be read only? Use const.
• Will the caller's data be modified? Use var.
• Is the data a small simple type that you need a local copy of? Use a value parameter (no keyword).

5. Name deliberately

• Procedures: ReadClientInfo, PrintServiceLines, ShowMenu — verb phrases.
• Functions: CalculateSubtotal, CalculateTax, CalculateLineTotal — verb + noun (what they compute).
• Boolean functions: IsValid, HasServices, IsOverdue — question form.

6. Move variables to the narrowest scope

The original had Choice and TempDesc as global variables, but they were used only inside the service-reading block. After refactoring, they are local to ReadServices. Local variables cannot accidentally interfere with other code.

7. Read the main body aloud

If the main body reads like an English summary of the program, the decomposition is good. Rosa's main body: "Read client info. Read services. Read payment terms. Calculate subtotal, tax, and grand total. Print header, service lines, and totals." Perfect.

Testing Individual Procedures

One of the biggest benefits of decomposition is testability. Rosa writes a small test program for her calculation functions:

program TestCalculations;
{ Quick sanity check for invoice calculation functions }

{ ... paste the type and const declarations and the calculation functions ... }

var
  TestHours: ServiceNumArray;
  TestRates: ServiceNumArray;
  Sub, Tax: Real;
begin
  { Set up known test data }
  TestHours[1] := 10.0;  TestRates[1] := 75.0;   { 750.00 }
  TestHours[2] := 5.0;   TestRates[2] := 100.0;   { 500.00 }
  TestHours[3] := 2.5;   TestRates[3] := 120.0;   { 300.00 }
  { Expected subtotal: 1550.00 }
  { Expected tax (8%): 124.00 }

  Sub := CalculateSubtotal(TestHours, TestRates, 3);
  Tax := CalculateTax(Sub);

  WriteLn('Subtotal: ', Sub:0:2, '  Expected: 1550.00');
  WriteLn('Tax:      ', Tax:0:2, '  Expected: 124.00');
  WriteLn('Total:    ', (Sub + Tax):0:2, '  Expected: 1674.00');

  if Abs(Sub - 1550.0) < 0.01 then
    WriteLn('PASS: Subtotal correct')
  else
    WriteLn('FAIL: Subtotal incorrect');

  if Abs(Tax - 124.0) < 0.01 then
    WriteLn('PASS: Tax correct')
  else
    WriteLn('FAIL: Tax incorrect');
end.

This kind of focused testing is impossible with a monolithic program — you would have to run the whole program and enter all the data by hand every time you wanted to check whether the calculation was right. With procedures, you extract the logic and test it in isolation.

Lessons Learned

1. Refactoring does not change behavior. The refactored program produces exactly the same output as the original. The improvement is internal: readability, maintainability, testability.

2. The main body is the table of contents. After refactoring, the main body tells you what the program does at a glance. Each procedure tells you how one piece is done.

3. Eliminate duplication ruthlessly. The line-total formula appeared twice. Now it appears once, as a function. This eliminates an entire class of bugs.

4. Variables should live in the narrowest possible scope. Choice and loop counters are local to the procedures that need them, not global to the entire program.

5. Parameter modes are documentation. A reader can tell from the heading whether a procedure reads data (const), writes data (var), or works with a local copy (value). This makes code review faster and more reliable.

6. Testability is a design goal, not an afterthought. By extracting pure computation into functions, Rosa can test her math independently of her I/O. This pays dividends every time she needs to change or extend the program.

Exercises for This Case Study

1. Rosa wants to add a discount feature: if the subtotal exceeds $2,000, apply a 5% discount before tax. Where should this logic go? Write the function and integrate it into the refactored program.

2. Write a procedure PrintServiceSummary that displays: total number of services, total hours worked, average hourly rate, and the most expensive service. All parameters should be const.

3. The current program uses a global constant TaxRate. Refactor it so the tax rate is read from the user (some clients are in different jurisdictions). Which procedure signatures need to change? What parameter mode should the tax rate use?

4. Rosa wants to save the invoice to a text file as well as displaying it on screen. Without knowing file I/O (that is Chapter 13), describe at the design level how you would modify the program. Which procedures would change? Would you need new procedures? Hint: Think about separating "generating output lines" from "where those lines go."