Classes in Clean Code: Small Classes, Cohesion and Organizing for Change

Version note: The code in this post was built and run with C# 14 and .NET 10.

Guess First: Same Cart, Two Prices

Ada sees a quote on the cart page, then places the order with the same cart. Both calls live inside the legacy OrderService, in the same class, a few lines apart:

var service = new OrderService();
var cart = new Dictionary<string, int> { ["978-0441013593"] = 1, ["978-0553380163"] = 1 };   // Dune + audiobook
decimal quote = service.GetQuote(1, cart, null, 1);
int id = service.ProcessData(1, cart, null, false, false, 1);
Console.WriteLine($"{DateTime.Now.DayOfWeek}: quote={quote} charged={service.GetOrder(id).Total}");

The cart holds Dune (450 TRY (Turkish lira)) and the audiobook edition of A Brief History of Time (250 TRY), Ada is a VIP (Very Important Person) customer, and there's no coupon. Is the quote shown on the screen the same as the amount charged to the card?

A Class's Size Is Measured in Responsibilities, Not Lines

I ran this post on a Saturday:

Saturday: quote=731.50 charged=690.52

The customer was shown 731.50 TRY and charged 690.52 TRY. This time in the customer's favor, but it could just as well have been the other way around. ProcessData applies a 10% discount to audiobooks and a 2% weekend discount; GetQuote knows about neither. Two copies of the pricing rule sit in the same class, in the same file, and they have still drifted apart. The problem isn't that the file is 600 lines long; it's that those 600 lines contain pieces of work that change without knowing about each other.

In the classes chapter of Clean Code (2008; second edition 2025), Robert C. Martin (Uncle Bob) measures a class's size in responsibilities, not lines. The practical test he suggests is about the name: can you describe what the class does in about 25 words, without using "and", "or" or "but"? Let's try it for OrderService: "It places orders and calculates quotes and cancels and ships and creates customers and produces sales reports and writes invoices and loads books from a CSV (Comma-Separated Values) file." Because its name is OrderService, it can carry all of it; everything has something to do with orders.

A more useful question is this: who asks for this class to change? Marketing changes the coupon and VIP discounts, accounting changes the tax rate and invoice format, logistics changes the shipping fee, customer service changes the cancellation rules, management changes the report columns, and the content team changes the CSV format. Six different teams, on six different schedules, commit to the same file. This is the "actor" concept in Martin's definition of SRP (Single Responsibility Principle); the principle itself, the actor definition and the fragmentation that happens when it is overdone will be covered in depth in the upcoming SOLID (Single Responsibility, Open/Closed, Liskov Substitution, Interface Segregation, Dependency Inversion) and Design Principles series post "Single Responsibility Principle: Not "One Thing" but One Actor". This post looks at something more mechanical: how to see in the code that responsibilities have come apart.

Seeing Cohesion Through Field Usage

Martin's measure of cohesion (how much the parts of a class belong together) is simple: in an ideal class every method uses most of the class's fields. If the methods don't share fields, the class is really several classes placed side by side. The LCOM (Lack of Cohesion of Methods) metric from Shyam Chidamber and Chris Kemerer's 1994 paper on object-oriented metrics is the numeric form of the same idea. Lining up the legacy class's methods by the state they use:

Of the six instance fields, pc is used by two methods and cache by two; most methods don't touch any instance field at all. What holds the class together isn't its own state but the static Db that everyone reaches into. You don't have to count this by hand; turn on CA1822, one of the CA (Code Analysis, .NET's built-in analyzer rules) rules, in .editorconfig and the compiler says the same thing:

OrderService.cs(194,24): warning CA1822: Member 'KargoUcretiHesapla' does not access instance data and can be marked as static (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1822)
OrderService.cs(241,22): warning CA1822: Member 'GetOrder' does not access instance data and can be marked as static (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1822)
OrderService.cs(249,28): warning CA1822: Member 'GetOrders' does not access instance data and can be marked as static (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1822)
OrderService.cs(269,21): warning CA1822: Member 'CheckStock' does not access instance data and can be marked as static (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1822)
OrderService.cs(281,21): warning CA1822: Member 'TryCalc' does not access instance data and can be marked as static (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1822)
OrderService.cs(314,20): warning CA1822: Member 'CalcPoints' does not access instance data and can be marked as static (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1822)
OrderService.cs(324,20): warning CA1822: Member 'CreateCustomer' does not access instance data and can be marked as static (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1822)
OrderService.cs(343,23): warning CA1822: Member 'Report' does not access instance data and can be marked as static (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1822)
OrderService.cs(407,20): warning CA1822: Member 'ImportBooks' does not access instance data and can be marked as static (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1822)
OrderService.cs(439,25): warning CA1822: Member 'OldCalcDiscount' does not access instance data and can be marked as static (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1822)
OrderService.cs(451,21): warning CA1822: Member 'SendMail' does not access instance data and can be marked as static (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1822)

Eleven of the twenty methods say "make me static". The purpose of CA1822 is actually a small performance gain, but it's a strong signal for cohesion: a method that doesn't touch instance state carries no reason to live inside that class. Turn on CA1506 (class coupling) and CA1502 (cyclomatic complexity) on the same file as well, with low thresholds set through CodeMetricsConfig.txt, and the picture is complete:

OrderService.cs(11,18): warning CA1506: 'OrderService' is coupled with '28' different types from '10' different namespaces.
OrderService.cs(31,20): warning CA1502: 'ProcessData' has a cyclomatic complexity of '27'.

Splitting Up OrderService

The last column of the table already gives the split plan. We gather the methods that share a reason to change into the same class:

• OrderPricing: unit price, coupon, VIP, volume and weekend discounts, shipping fee, tax; both quotes and orders use it.
• OrderPlacement: customer and stock checks, payment, decrementing stock (in its Result<T>-returning form from the seventh post).
• OrderNotifier: the text and delivery of confirmation, cancellation and shipping emails.
• OrderCancellation, ShippingLabelService, InvoiceGenerator, SalesReport, CustomerRegistration and the BookCsvImporter from the seventh post.

Pricing first, because it's the source of the opening bug:

// A single place computes the price: for both the cart quote and the order
public sealed class OrderPricing(TimeProvider clock)
{
    private const decimal TaxRate = 0.10m;

    public PriceQuote Quote(IReadOnlyList<CartLine> lines, CustomerTier tier, string? couponCode, ShippingMethod shipping, string city)
    {
        decimal subtotal = lines.Sum(l => UnitPrice(l.Book) * l.Quantity);
        decimal discount = CouponDiscount(couponCode, subtotal) + TierDiscount(tier, subtotal)
            + VolumeDiscount(lines.Count) + WeekendDiscount(subtotal);
        decimal net = subtotal - discount;
        decimal fee = ShippingFee(shipping, net, city);
        decimal tax = net * TaxRate;
        return new PriceQuote(subtotal, discount, fee, tax, Math.Round(net + tax + fee, 2));
    }

    private static decimal UnitPrice(Book book) => book.Format switch
    {
        BookFormat.Ebook => book.Price * 0.8m,
        BookFormat.Audio => book.Price * 0.9m,
        _ => book.Price
    };

    private static decimal CouponDiscount(string? code, decimal subtotal) => code?.ToUpperInvariant() switch
    {
        null or "" => 0m,
        "WELCOME10" => subtotal * 0.10m,
        "SUMMER25" => 25m,
        _ => throw new ArgumentException($"Unknown coupon '{code}'.", nameof(code))
    };

    private static decimal TierDiscount(CustomerTier tier, decimal subtotal) => tier == CustomerTier.Vip ? subtotal * 0.05m : 0m;

    private static decimal VolumeDiscount(int lineCount) => lineCount > 5 ? 15m : 0m;

    private decimal WeekendDiscount(decimal subtotal) =>
        clock.GetLocalNow().DayOfWeek is DayOfWeek.Saturday or DayOfWeek.Sunday ? subtotal * 0.02m : 0m;

    private static decimal ShippingFee(ShippingMethod method, decimal amount, string city) => method switch
    {
        ShippingMethod.Standard when amount >= 500m => 0m,
        ShippingMethod.Standard => city == "Istanbul" ? 29.90m : 39.90m,
        ShippingMethod.Express => 89.90m,
        ShippingMethod.StorePickup => 0m,
        _ => throw new ArgumentOutOfRangeException(nameof(method), method, null)
    };
}

var pricing = new OrderPricing(TimeProvider.System);
PriceQuote quote = pricing.Quote(cart, CustomerTier.Vip, null, ShippingMethod.Standard, "Istanbul");
Console.WriteLine($"{DateTime.Now.DayOfWeek}: {quote}");

Saturday: PriceQuote { Subtotal = 675.0, Discount = 47.250, Shipping = 0, Tax = 62.77500, Total = 690.52 }

The total matches what the legacy code charged: 690.52. The quote screen and the order both call this single method now, so the two numbers can't drift apart. Every private method in the class answers a single question, and its only field is clock, for the weekend rule. When marketing asks for a new coupon, it's clear which file to open, and that file contains no invoice or CSV code. TimeProvider is here on purpose: thanks to this clock abstraction, added to the BCL (Base Class Library) in .NET 8, the weekend rule can be tested without waiting for Saturday; how to do that is the topic of the next post.

The notification class is small for the same reason:

public sealed class OrderNotifier(IEmailSender sender)
{
    public Task SendConfirmationAsync(OrderConfirmation order, CancellationToken ct = default) =>
        sender.SendAsync(new EmailMessage(order.CustomerEmail, $"Order #{order.OrderId}", ConfirmationBody(order)), ct);

    private static string ConfirmationBody(OrderConfirmation order)
    {
        var body = new StringBuilder();
        body.AppendLine(CultureInfo.InvariantCulture, $"Dear {order.CustomerName},");
        body.AppendLine(CultureInfo.InvariantCulture, $"Your order #{order.OrderId} has been received.");
        foreach (ConfirmationLine line in order.Lines)
            body.AppendLine(CultureInfo.InvariantCulture, $" - {line.Title} x{line.Quantity} = {line.LineTotal:0.00}");
        body.AppendLine(CultureInfo.InvariantCulture, $"Shipping: {order.Shipping:0.00}");
        body.AppendLine(CultureInfo.InvariantCulture, $"Total: {order.Total:0.00} TRY");
        return body.ToString();
    }
}

MAIL to ada@example.com: Order #1000
Dear Ada Lovelace,
Your order #1000 has been received.
 - Dune x1 = 450.00
 - A Brief History of Time x1 = 225.00
Shipping: 0.00
Total: 690.52 TRY

Notice that OrderNotifier doesn't take the Order entity but an OrderConfirmation record carrying only the data it needs; when the email text changes, you don't touch the order's model. Measured with the same CA1502 and CA1506 thresholds, OrderPricing comes out coupled to 18 types and OrderNotifier to 12, and the most complex method has a cyclomatic complexity of 2. Most of that coupling count is BCL types such as decimal, Enumerable and Func<T, TResult>; the metric counts them too, so read the numbers as a direction, not an absolute target.

.NET itself splits big jobs into small classes, too. HttpClient sends requests, the HttpMessageHandler chain handles the connection, and DelegatingHandler derivatives take on cross-cutting work such as retries or logging. In System.Text.Json, Utf8JsonReader reads, Utf8JsonWriter writes, and JsonSerializer uses both to convert to objects. Each of them changes for a single reason.

Building Classes with Modern C#

The most natural tool for small classes, which came with C# 12, is the primary constructor. It's ideal for a class that takes its dependencies and configuration and uses them in its methods; OrderPricing(TimeProvider clock) and OrderNotifier(IEmailSender sender) are like that. There's one trap to know about: a primary constructor parameter isn't readonly, and if you also copy it into a field you carry two separate copies of the same value:

public sealed class StockCounter(int initial)
{
    private readonly int _initial = initial;

    public int Remaining => initial;

    public void Reset() => initial = _initial - 1;
}

Program.cs(25,41): warning CS9124: Parameter 'int initial' is captured into the state of the enclosing type and its value is also used to initialize a field, property, or event.

The assignment inside Reset compiles; the parameter lives inside the class like an ordinary, mutable variable. The rule is simple: either use the parameter directly or assign it to a readonly field, not both. The compiler's CS9124 warning catches exactly this confusion.

The second tool is sealed. Every class in this series is sealed, because leaving a class that wasn't designed for inheritance open means promising an extension point nobody thought about. If inheritance is really needed, removing sealed is a backward-compatible change; the reverse isn't. On top of that, the JIT (Just-in-Time compiler) can more easily devirtualize virtual calls and type checks on sealed types. For internal types CA1852 reminds you:

Program.cs(18,20): warning CA1852: Type 'InvoiceNumberFormatter' can be sealed because it has no subtypes in its containing assembly and is not externally visible (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1852)

The third is building valid objects with required and init. Every property of the legacy Book is { get; set; }; you can build a book without an ISBN (International Standard Book Number), and change its price from anywhere after it's built. In the new version:

public sealed class Book
{
    public required string Isbn { get; init; }
    public required string Title { get; init; }
    public required decimal Price { get; init; }
    public BookFormat Format { get; init; } = BookFormat.Print;
}

An object initializer that forgets the ISBN no longer compiles:

var book = new Book { Title = "Dune", Price = 450m };

Program.cs(3,16): error CS9035: Required member 'Book.Isbn' must be set in the object initializer or attribute constructor.

required says "the object can't be built without this value", init says "it doesn't change after construction". If validation is needed (a negative price, say), a constructor or the value objects from the sixth post give a stronger guarantee; required only checks whether a value was assigned.

Finally, static. Not every method CA1822 points at is bad: a stateless, pure calculation like OrderPricing.UnitPrice sits perfectly well inside the class as private static. What's bad is static state: Db, Mailer, LastError, cnt. A pure static function is testable and shareable; static state invisibly ties every class to every other.

Manager, Helper, Util: Dumping-Ground Classes

The easiest mistake while splitting OrderService is to put the methods you can't place into OrderHelper or CommonUtils. These names describe a location, not a responsibility: "things that didn't fit anywhere else". Six months later OrderHelper holds an invoice number format, phone validation and an SQL (Structured Query Language) query side by side, and a new OrderService has been born.

The name test helps here too. If you're about to put a method into a Helper, ask what it's about and use that concept's name: the invoice number is the job of InvoiceNumber, phone validation the job of a PhoneNumber value object. The same goes for Manager: nobody can tell the difference between OrderManager and OrderService. Each of the class names in this post (OrderPricing, OrderNotifier, OrderCancellation) says what it does and what it doesn't do at the same time.

Common Pitfalls

Moving every method into its own class. CouponDiscountCalculator, TierDiscountCalculator, VolumeDiscountCalculator and a DiscountCalculatorAggregator that combines them spread a single reason to change across four files; when marketing asks for a new rule, you open four files. The measure of cohesion is still the same: what changes together should stay together. The "shallow module" critique in John Ousterhout's A Philosophy of Software Design (2018) targets exactly this excess; we'll come back to that debate in the thirteenth post.

Splitting by file but keeping the shared state. If the new classes all still reach into the static Db, you've only cut OrderService into six files; the coupling stays the same. Each class should take its dependencies through its constructor, and a DI (Dependency Injection) container should wire them up. The mechanics are in the C# Advanced series post "Dependency Injection Basics in C#: Lifetimes, the Container and Captive Dependencies".

Splitting classes by technical layer. A split such as OrderValidator, OrderCalculator, OrderPersister can force you to change three classes together for every business change. Draw the line according to who asks for the change.

Forgetting the Order entity itself. As we saw in the sixth post, some rules (adding lines, the total) belong not in a service but in the object that owns the data. If you move everything into new service classes, you end up with an anemic domain model.

When Not to Use It

Don't split a small, stable class just because it's long. If a 300-line InvoicePdfRenderer does one thing and only accounting changes it, cutting it into five makes the reader's job harder; line count isn't responsibility count.

Don't design five classes up front for a prototype or a one-week internal tool. Write one working class first; split it when a second actor asks for a change. The best time to split is the first moment you touch the same file for two different reasons.

Don't use primary constructors everywhere. In a class that has to validate its parameters or has more than one constructor, a classic constructor reads more clearly. And required isn't domain validation; it's for mandatory fields that accept any value.

A Small Experiment

★☆☆ Move the legacy FaturaOlustur into an InvoiceGenerator class. Instead of recomputing tax itself, have it use the PriceQuote from OrderPricing; in the legacy code, was the tax on the invoice computed with the same formula as the tax on the order?

★★☆ Write OrderCancellation: refund, returning stock, deducting points and notification. Which dependencies does it take through its constructor? Does the point deduction rule belong to this class or to Customer?

★★★ Set CA1822, CA1502 and CA1506 to warning across the whole project and set the thresholds with CodeMetricsConfig.txt. How many warnings does the legacy file produce, and how many does the split version? Which warnings would you turn off on purpose?

In this post we split OrderService along its reasons to change and gathered the pricing rule in one place. OrderPricing's weekend rule leaves a question open, though: how do you test this code on a day that isn't Saturday? The next post looks at test code itself: the F.I.R.S.T. (Fast, Independent, Repeatable, Self-Validating, Timely) rules, readable asserts, test data builders, and controlling the clock with TimeProvider.