Case Study: GlobalBank — Code Review of Legacy ACCT-MAINT

Background

ACCT-MAINT is GlobalBank's account maintenance program — the workhorse that handles account opens, closes, address changes, status updates, and balance adjustments. Originally written in 1992, it has been modified by at least fifteen developers over its lifetime. At 4,200 lines, it is the second-largest program in GLOBALBANK-CORE, second only to TXN-PROC (transaction processing).

Despite its critical role — ACCT-MAINT processes every account change in the bank — it had never undergone a formal code review. Changes were reviewed informally ("Hey Maria, can you look at this?") but never against a checklist or with documented findings.

When Priya Kapoor, the systems architect, proposed modernizing ACCT-MAINT to support a new online banking interface, she first wanted to understand the code's quality baseline. She asked Maria Chen and Derek Washington to conduct a formal review.

The Review Process

Maria and Derek spent three days on the review:

Day 1: Maria walked Derek through the program's structure, explaining the business logic, the file layouts, and the modification history she knew from memory. Derek took notes and mapped the program's control flow.

Day 2: Both reviewed independently against the code review checklist, documenting findings with line numbers and severity.

Day 3: They met to reconcile their findings, resolve disagreements, and prioritize the issues.

Key Findings

The 187-Line Paragraph

The most striking finding was paragraph 2000-PROCESS-MAINTENANCE, which had grown to 187 lines over thirty years. It contained an EVALUATE with 12 WHEN clauses, each with nested IFs up to 7 levels deep. Its cyclomatic complexity was 47.

Derek counted the number of independent paths: at least 47, meaning a complete test suite would need a minimum of 47 test cases just for this one paragraph. The existing "test" — running the program with 5 sample transactions — covered perhaps 8 of those paths.

Maria explained how it got this way: "Every new maintenance type was added as another WHEN clause. Nobody wanted to restructure the paragraph because nobody fully understood it. So it just grew."

The Silent VSAM Error

Line 1456 contained a VSAM WRITE without a FILE STATUS check:

           WRITE ACCT-MASTER-REC
           MOVE "UPDATED" TO WS-STATUS.

If the WRITE failed (duplicate key, out of space, I/O error), the program would set the status to "UPDATED" regardless. In the worst case, a customer's account change would appear to succeed but would not actually be written. Maria turned pale when she saw this — she estimated this code path was executed 50,000 times per day.

The Scope Terminator Bug

Line 3012 contained a subtle scope terminator error that had been introduced during a 2018 modification:

           IF ACCT-TYPE = "CD"
               IF ACCT-MATURITY-DATE < WS-CURRENT-DATE
                   MOVE "MATURED" TO ACCT-STATUS.
               MOVE "Y" TO WS-NOTIFY-FLAG.
           ELSE
               MOVE "N" TO WS-NOTIFY-FLAG.

The period after ACCT-STATUS terminated both IF statements. The MOVE "Y" TO WS-NOTIFY-FLAG executed for ALL accounts, not just matured CDs. The ELSE clause was associated with no IF at all — it was dead code that never executed. The result: every account processed after a CD got WS-NOTIFY-FLAG set to "Y", triggering unnecessary customer notifications.

This bug had been in production for seven years. Nobody noticed because the notification system had a separate filter that caught most of the false positives. But some notifications had been going out erroneously — a "minor annoyance" that customer service had been handling manually.

The Phantom Paragraphs

Three paragraphs — 2500-PROCESS-LIEN, 2600-PROCESS-GARNISHMENT, and 2700-PROCESS-LEVY — were defined but never PERFORMed. They totaled 94 lines. Maria remembered: "Those were added in 2004 for a legal hold feature that was later implemented in a separate program. Nobody removed them."

Impact Assessment

Maria and Derek estimated the total impact:

  • The silent VSAM error had not (yet) caused a visible production incident, but represented an existential risk. A single file-full condition during peak processing could lose thousands of account updates.
  • The scope terminator bug had caused approximately 12,000 erroneous notifications over seven years, each costing customer service approximately 3 minutes to handle — roughly 600 person-hours of wasted effort.
  • The 187-line paragraph did not contain a specific bug, but made every modification to the program risky. Developer time spent understanding the paragraph before making changes was estimated at 40+ hours per year.

Remediation

Phase 1 (critical fixes) was completed in one week. Phase 2 (refactoring the 187-line paragraph into 12 focused paragraphs averaging 15 lines each) took three weeks. The refactored program was 3,650 lines — 550 lines shorter despite better documentation.

The unit test suite (built per Chapter 34's practices) now covers 64 of the 47 paths through the original paragraph (the refactored version has more, smaller paragraphs with lower individual complexity).

Discussion Questions

  1. The scope terminator bug at line 3012 went undetected for seven years. What combination of code review and testing practices would have caught it at introduction?

  2. The 187-line paragraph had a cyclomatic complexity of 47. Using the guidelines from Section 35.5, how would you plan the refactoring? What paragraph boundaries would you draw?

  3. Should the three phantom paragraphs (2500, 2600, 2700) have been removed immediately, or left in place until the next major modification? Justify your position.

  4. Maria estimated 40+ hours per year of developer time spent understanding the complex paragraph. Over the program's remaining lifetime (potentially decades), what is the total cost of NOT refactoring?