This module's four previous lessons gave BiblioTech a consistent style, a vocabulary of design patterns, dependencies decoupled through interfaces, and a battery of unit tests that verify its behavior with no need for a real database. This last lesson of Module 8 closes the loop: it teaches what a good code review looks for, catalogs the most common code smells (signals of a design that could be improved), and refactors a Library method that mixes several responsibilities — leaning, precisely, on the previous lesson's unit tests as a safety net. With this, BiblioTech reaches Module 9 (Final Project) with code that is not only functional, but reviewed, tested, and cleaned up according to everything covered in this module.

1. What a good code review looks for
2. Review checklist
3. Common code smells
4. Refactoring: extract method and extract class
5. Unit tests as a safety net before refactoring
6. Example: refactoring a long Library method
7. Closing out Module 8 and the link to the Final Project

A code review is the process of having someone else (or, on a solo project, yourself with fresh eyes) read a change before it's permanently merged into the project, looking for problems the original author, immersed in the details, may not have noticed. A good review doesn't try to impose personal style preferences — EditorConfig (Lesson 1) already automates that — but instead verifies four concrete aspects:

A review focused on these four aspects, rather than on personal style preferences, is faster, more objective, and creates less friction between whoever writes the code and whoever reviews it.

A concrete checklist helps ensure a review doesn't rely solely on the reviewer's intuition. A reasonable checklist for a change to BiblioTech:

• [ ] Does the name of every new method and variable follow Lesson 1's conventions (PascalCase/camelCase, I/_ prefixes, Async suffix)?
• [ ] Is there any method doing more than one clearly separable thing (validating, persisting, notifying...)?
• [ ] Are external dependencies (persistence, services) received through injection, or created directly with new inside the class (Lesson 3)?
• [ ] Does the change include new unit tests, or modify existing tests that no longer reflect the new behavior (Lesson 4)?
• [ ] Does nullability (?) faithfully reflect what can and can't return null?
• [ ] Do comments explain non-obvious decisions, or do they just repeat what the code already says (Lesson 1)?

This list isn't exhaustive or universal — each team adjusts its own over time — but it serves as a concrete starting point, more useful than a review with no explicit criteria at all.

A code smell is a surface-level signal in the code that suggests, without being a bug in itself, that the underlying design could be improved. Four especially common code smells:

None of these smells is, by itself, an error that stops the program from compiling or running — hence the name "smells," not "errors" — they're signals worth investigating, not absolute rules that always demand immediate refactoring.

Refactoring means changing the code's internal structure without changing its observable behavior: the program keeps doing exactly the same thing from the outside, but its internal code ends up better organized. Two refactoring techniques cover most of the previous section's smells:

• Extract method: take a fragment of a long method and turn it into its own method, with a name that explains what that fragment does. Directly resolves the "long method" smell.
• Extract class: when a class mixes several responsibilities (the "too many responsibilities" smell), move part of its members into a new class dedicated to that responsibility — exactly what Lesson 3 already did when it extracted ILibraryRepository and its implementations out of Library.

DescribeLoan doesn't change the message shown on the console — the observable behavior is identical — but it now has a name that explains its purpose, and it can be reused anywhere else in the program that needs the same format, with no duplicated logic.

The question that should always come up before refactoring is: how do I know I haven't broken anything? With no tests, the only answer is "by running the whole application by hand and trusting nothing was overlooked" — slow, and unreliable. With Lesson 4's unit tests already written for Loan.RegisterReturn() and Library.LendBookAsync, refactoring stops being a leap of faith:

This is exactly the role unit tests play as a safety net: they don't prevent a mistake while refactoring, but they catch it immediately — in seconds, by rerunning the same test suite — instead of it being discovered much later, perhaps already in production. Refactoring code with no test backing it isn't impossible, but it's considerably riskier: every change depends solely on the programmer's attention at that moment.

Imagine that, in the rush of adding functionality module by module, Library ended up with a method that mixes validation, loan registration, and notification, all together:

This method works, and the previous lesson's tests would probably already cover it with a minor tweak — but it mixes four distinct responsibilities into a single block of code (validating, waiting, registering+persisting, showing a message), which makes it hard to read at a glance and hard to test in isolation. Applying "extract method" to each block:

The observable behavior hasn't changed at all: the same exceptions are thrown in the same cases, the same message is shown on the console, the same event fires, and the same repository is invoked. What's changed is that ManageLoanAsync now reads almost like a list of named steps (ValidateLoan, RegisterAndPersistLoan), and each of those steps could be tested separately if more granularity is needed in the future. Rerunning Lesson 4's tests (LendBookAsync_WithAvailableBook_SavesCatalog and LendBookAsync_WithUnavailableBook_ThrowsAndDoesNotSave, adapted to the method's new name) against this refactored version, they should still pass exactly as before — that's the concrete confirmation that the refactoring was safe.

With this lesson, Module 8 (Best Practices and Design Patterns) comes to a close. Across its five lessons, BiblioTech hasn't gained a single new domain feature — this module's explicit goal was a different one: to take a step back and consolidate everything built in the previous modules. The full journey:

Module 9 (Final Project) now picks up everything built throughout the course — the complete domain since Module 2, Module 5's persistence, Module 7's five interfaces, and this Module 8's practices — to build BiblioTech's final, complete version: its requirements will be precisely defined, its implementation planned, it will be built following the standards and patterns already learned, tested with the same unit-testing discipline seen here, and finally deployed. Nothing learned in this module is set aside: it's precisely the set of practices that final version will be built with.

• Refactoring with no test backing the change: with no prior tests, there's no objective way to confirm behavior didn't change; in that case, write at least the most important tests for the current behavior first, and refactor afterward.
• Changing behavior "along the way" while refactoring: refactoring and fixing a real bug are two different activities; mixing them in the same change makes it harder to tell, if something fails afterward, whether the refactoring or the fix was the cause.
• Reviewing code while only looking at style: a review focused solely on spacing, names, or formatting (automatable with EditorConfig, Lesson 1) wastes the opportunity to catch design problems, missing tests, or real risks.
• Extracting methods to the extreme: splitting a method into fragments so small that understanding a simple flow requires jumping between ten different methods also hurts readability; the goal is clarity, not fragmentation for its own sake.
• Tip: if you're unsure whether a method needs refactoring, ask yourself whether you could explain it in one short sentence; if the answer needs "and also..." several times, it probably mixes more than one responsibility.

1. In the "before" version of ManageLoanAsync from section 6, identify which code smell from section 3 best describes its main problem, and explain in one sentence why.

2. The following Member method mixes registering a penalty with showing a console message. Refactor it with "extract method," separating both responsibilities:

   public void ApplyPenalty(decimal amount)
   {
       OutstandingBalance += amount;
       Console.WriteLine($"A penalty of {amount:C} was applied to {Name}. Outstanding balance: {OutstandingBalance:C}");
   }
   

The main code smell is long method (with an additional mixed responsibility at the method level, "too many responsibilities"): a single method validates, waits, registers, persists, and notifies, all in the same block of code, making it hard to read and test in parts.

public void ApplyPenalty(decimal amount)
{
    RegisterPenalty(amount);
    NotifyPenalty(amount);
}

private void RegisterPenalty(decimal amount)
{
    OutstandingBalance += amount;
}

private void NotifyPenalty(decimal amount)
{
    Console.WriteLine($"A penalty of {amount:C} was applied to {Name}. Outstanding balance: {OutstandingBalance:C}");
}

In this lesson you've seen what a good code review looks for (readability, tests, adherence to standards, design), a concrete checklist for applying it, the most common code smells (long method, duplication, classes with too many responsibilities, excessive parameters), two refactoring techniques (extract method and extract class), and how the previous lesson's unit tests act as a safety net while refactoring a long Library method with no change to its observable behavior.

With this, Module 8 is complete. BiblioTech reaches Module 9 — the course's Final Project — with a solid domain, decoupled persistence, a battery of unit tests, and code reviewed according to the standards and patterns learned across these five lessons: everything needed to build, plan, test, and deploy BiblioTech's complete, definitive version.