A bloated class in the wild

The ReconciliationIntro class in a small command-line accounting tool started as a simple entry point: display a few messages and hand off to the classes that do real work. Over time, file-loading logic, data reconciliation, and output handling all crept in. The result is a class that is difficult to reason about, hard to unit-test due to heavily nested private methods, and painful to extend — adding support for a third credit card would mean duplicating a fourth near-identical code path.

The goal is to prepare this code for a strategy-pattern-based redesign that treats each bank and credit card account uniformly. Before that can happen, the class itself must shrink. The refactoring targets four distinct problems, tackled in order: the oversized class, the untestable private code, the single method doing too much, and the repeated structural patterns with varying details.

The payoff for splitting the class is concrete: clear separation of concerns, smaller isolated contexts that fit in your head, and localised state changes that won't ripple across the system. When a bug needs fixing or a feature needs adding, the location to look should be obvious.

Refactoring discipline

Working in tiny steps is the single most important habit when refactoring. At every step the code must compile and all tests must pass. If a change breaks a test, the cause is identifiable and reversible within seconds. Small commits keep the current state of the work in your head, and prevent the rabbit-hole scenario where the code won't compile and even the tests can't be run.

This requires test coverage before starting. The code in question already has coverage, though making it more testable is itself a future goal. One refactoring principle stands out: never combine moves and renames with edits. Each commit should do one thing only.

Step one: rearrange with regions

The first move is cosmetic but enabling. All methods in ReconciliationIntro are grouped into logical regions using C# preprocessor directives. This is a pure rearrangement with no behavioural change, and it makes the code's implicit contexts visible. After grouping, the file-loading methods form the largest and most duplicated cluster. That's where the pain is greatest and where the extraction should begin.

A new FileLoader class will be carved out of that cluster. The other groups will also become separate classes eventually, but they are simpler and will be handled later — the article's focus stays on the file-loading code.

Mapping the way to smaller classes

After grouping methods into regions and drawing the relationships between file-loading methods, you can see the code isn't a strictly self-contained context. Some methods you want to move call back out to methods you plan to keep behind. Two ways to handle this are making the staying methods public so the new class can call them, or moving client calls out of the file-loading code into other methods that stay behind.

Before doing anything else, lay out a plan of action. The goal is to break the large ReconciliationIntro class into a pared-down version plus five new classes. There's no one-to-one mapping between regions and classes; later refinement will split first groupings into finer-grained boundaries.

Steps proceed in this order:

  1. Rearrange all methods into sensible groupings.
  2. Analyse relationships between file-loading methods and the rest of the class.
  3. Modify the methods that are staying behind, since they're called by code that will move.
  4. Create covering tests for the new FileLoader class by moving existing tests into a new FileLoaderTests class.
  5. Create the new FileLoader class and move just two methods first (a public method and a private method it calls).
  6. Move other file-loading methods to FileLoader.
  7. Extract more new classes for other code regions.

Working in such tiny steps is the point: each change sets up the next one to be small and simple. You're making small changes to facilitate more small changes—"Make the change easy, then make the easy change."

Preparing the methods that stay behind

Two staying-behind methods are called by file-loading code: Set_path and Recursively_ask_for_budgeting_months. Handle each according to how tightly coupled it is to the file-loading code.

Recursively_ask_for_budgeting_months gets option one: make it public so the new class can call back to it. In fact it already is public, because it was made public for testing purposes—itself a code smell suggesting it belongs on the public interface of its own class.

Set_path gets option two: call it separately from the file-loading code and pass the result in as a parameter, because it changes an internal path variable.

Proceed through four compile-safe commits. The method Create_pending_csvs is the caller to change:

  1. Give Create_pending_csvs a parameter with a default value so the code still compiles.
  2. Call Set_path separately before calling Create_pending_csvs, and pass the updated path member variable in as the parameter.
  3. Remove the Set_path call from inside Create_pending_csvs, and use the passed-in value instead of the member variable.
  4. Remove the default parameter value, forcing all clients to pass one.

Ordering the steps this way keeps the code compiling at every stage. Changes could later be squashed into fewer commits, but the small commits make each step clearer.

private void Create_pending_csvs()
{
  // Some code
}
⇓ 
private void Create_pending_csvs(string path = "")
{
  // Some code
}

case "1":
{
  Create_pending_csvs();
}
break;
⇓ 
case "1":
{
  Set_path();
  Create_pending_csvs(_path);
}
break;

private void Create_pending_csvs(string path = "")
{
  try
  {
    Set_path();
    var pending_csv_file_creator = new PendingCsvFileCreator(_path);
⇓ 
private void Create_pending_csvs(string path = "")
{
  try
  {
    Set_path();
    var pending_csv_file_creator = new PendingCsvFileCreator(path);

private void Create_pending_csvs(string path = "")
{
  // Some code
}
⇓ 
private void Create_pending_csvs(string path = "")
{
  // Some code
}

Moving the First Method and Its Caller

With a dedicated test class in place, the first target for extraction is Bank_and_bank_out__Merge_bespoke_data_with_pending_file and its private callee, Bank_and_bank_out__Add_most_recent_credit_card_direct_debits. The private method sits at the leaf end of the call chain and has no independent tests of its own; it is exercised through its public caller. Moving the caller first, then the callee, would leave the tree awkwardly split across classes, so both are extracted together.

An important note on testing during refactoring: writing new tests is not automatically required. Because the functionality is unchanged, the job is to move, not create, the relevant coverage. The test M_MergeBespokeDataWithPendingFile_WillAddMostRecentCredCardDirectDebits verifies that new direct debit data merges correctly into a pending transaction file. It is one of the existing tests that will follow the extracted code into the new class.

While working on this method, it is worth re-examining its existing test for readability. One helper, Assert_direct_debit_details_are_correct, uses the vague word "correct" and deserves a clearer name or a restructured body. Readable tests serve as system documentation, so this kind of cleanup fits naturally into the migration work. The exact refactors made to that test are recorded in commits f090f26 and 6a6cece.

Extraction Procedure in Small Steps

The move is executed incrementally, keeping the code building and all tests green after each step. The sequence for moving a method pair looks like this:

  1. The new file-loading method is created in the destination FileLoader class. Since it will remain private in the final structure, it must be temporarily made public so the old caller can invoke it. Commit 0341476 shows this state.
  2. The ReconciliationIntro class instantiates FileLoader and calls its new public method. The old private method is deleted from ReconciliationIntro.
  3. The caller method is copied into FileLoader, leaving both versions in place for the moment. Duplication is expected and temporary.
  4. The covering test class is updated to instantiate FileLoader instead of ReconciliationIntro. Since the private leaf traveled with its caller, the tests pass against the new copy.
  5. The ReconciliationIntro method now delegates to the new FileLoader method.
  6. The moved method is made private again, the redundant caller is deleted from ReconciliationIntro, and the now-obsolete entries in the old test class are removed.

This manual procedure works because only two methods and their associated tests were in play. The same pattern applies to the remaining load-bearing methods in ReconciliationIntro.

Handling the Remaining Methods

Every other method slated for extraction follows the same outline: create a public copy in FileLoader, redirect the caller, relocate the tests, and delete the originals. Each move should be considered individually to catch dependencies and improve the APIs:

  • Do method names still describe what they do in their new home?
  • Should any parameter list be replaced with a structured object?
  • Are any parameters redundant now that the surrounding code has shifted?
  • Do nested callees alter state, and does that still make sense after the move?

It is tempting to flatten or inline the lower-level methods before extracting them, but doing so would break public callers and their tests. So each method is moved whole. Methods at the ends of the chain without direct test coverage are the simplest transfers; making them properly testable is one motivation for the extraction in the first place.

The batch of commits from 7cd53f6 through 7ab95f2 tracks those migration steps. They are not individually committed here, but the same build-and-test checkpoint applies after each move.

From Regions to Real Classes

After the initial region-based reorganization, the remaining methods in ReconciliationIntro still formed a clump that was hard to hold in your head. Drawing out the call hierarchy on a spreadsheet made the natural divisions visible: the code fell into three self-contained areas — User instructions, Gathering file / path info, and Debug mode switching code — not the two regions I had originally guessed at.

With that picture in hand, I deleted the old regions and replaced them with four new ones, rearranging methods so each would map cleanly onto a prospective class. This time the regions translated into three new extracted classes: Communicator, PathSetter, and DebugModeSwitcher. (The earlier extractions of FileLoader and BudgetingMonthService happened before this diagram was drawn, so they don’t appear in it.)

Extracting Independently

The new classes were pulled out gradually and safely, with the same discipline used for BudgetingMonthService: each had a single public entry point, and I removed the temporary regions once the extraction was complete. One thing I deliberately avoided was the anti-pattern of automatically injecting each extracted class into the constructor of the original ReconciliationIntro. The goal is for the extracted classes to stand alone and be genuinely independent, not to accumulate as constructor baggage on the already-bloated original.

PathSetter turned out to be the non-trivial one. The path-setting logic was already flagged as slightly tortuous during the earlier steps, and giving it its own class with a clear context was an improvement in itself — but it will still need further attention later. Extraction alone doesn’t fix convoluted code; it just makes the mess easier to see and work on.

The Result

At the end of the process, ReconciliationIntro went from 41 methods down to exactly three: Start, Reconciliate, and Do_matching. The class now delegates to several smaller, focused collaborators instead of trying to do everything itself.

Where This Leaves Us

This article stops in the middle of the refactoring work. If you want to inspect the code at this state, check out the commit 6103f0b. Most of the final extraction step described above happened after that commit, but the important checkpoint is clear: the file-loading code now lives in its own FileLoader class, which exposes the next set of problems.

Four methods in FileLoader are obviously problematic:

  • Load_bank_and_bank_in
  • Load_bank_and_bank_out
  • Load_cred_card1_and_cred_card1_in_out
  • Load_cred_card2_and_cred_card2_in_out

All four suffer from the same issues: they contain heavy duplication and look nearly identical at a glance, they are far too long, and they internally construct objects that they pass into one another in an intertwined, untestable fashion. These need tests before any further refactoring can safely proceed — that’s the natural next step, and it’s left here as an exercise for the reader until a follow-up article can cover it.