Formatting and File Layout: Making Code Read Like a Newspaper

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

Guess First

A colleague who moved the e-mail block at the end of the legacy ProcessData into its own class also wanted to log that the confirmation e-mail was sent, and added one line:

namespace Bookstore.Ordering;

public sealed class ConfirmationSender(TextWriter log)
{
    public void Send(string email, int orderId, bool sendMail)
    {
        if (sendMail)
            log.WriteLine($"MAIL to {email}: Order #{orderId}");
            log.WriteLine($"Confirmation logged for {email}");
        log.WriteLine("Done");
    }
}

The calling code doesn't want to send an e-mail:

using Bookstore.Ordering;

var sender = new ConfirmationSender(Console.Out);
sender.Send("ada@example.com", 1000, sendMail: false);

What does the program print? The two lines below if (sendMail) are aligned; both look like they depend on the condition.

Indentation Can Lie

The real output:

Confirmation logged for ada@example.com
Done

No e-mail was sent, yet the log says "confirmation logged". In C#, an if without braces covers only the first statement after it; the second line always runs, whatever its indentation says. The compiler doesn't see a problem: the build passed with zero warnings. Indentation means nothing to the compiler and is the only clue the reader has. That's why wrong indentation is as dangerous as a wrong comment.

Two things catch this kind of bug. With SonarAnalyzer (SonarSource's Roslyn analyzer package for C#) enabled, the build said:

ConfirmationSender.cs(9,13): warning S2681: This line will not be executed conditionally; only the first line of this 2-line block will be. The rest will execute unconditionally.

The second is dotnet format. It recalculates the indentation from the code's real structure and makes the lie visible:

$ dotnet format whitespace --verify-no-changes
ConfirmationSender.cs(9,9): error WHITESPACE: Fix whitespace formatting. Delete 4 characters. [P05a.csproj]
$ dotnet format whitespace
$ diff before.cs ConfirmationSender.cs
9c9
<             log.WriteLine($"Confirmation logged for {email}");
---
>         log.WriteLine($"Confirmation logged for {email}");

Once the formatter pulled the line back, it became clear that the second line never belonged to the if. The actual fix is a separate decision: should the log line go inside the if or stay outside? The formatter can't know. That's why the .NET runtime team's coding style document requires braces unless every branch is a single line; teams that use braces even on single-line ifs never run into this bug.

The Purpose of Formatting: Communication

Robert C. Martin (author of Clean Code) treats formatting not as a matter of aesthetics but as communication. His reasoning in the 2008 edition: the functionality you write today will probably change in the next release, but its readability keeps affecting every change after that. The Formatting chapter of the second edition opens with the same heading, "The Purpose of Formatting". Its other headings are vertical and horizontal formatting, team rules and Martin's own rules.

Martin's best-known image is the newspaper metaphor: a good source file reads like a newspaper article. The headline (the class name) tells you whether you're in the right place. The first paragraph (the public methods) gives you the synopsis of the story. Detail increases as you go down, and the lowest-level helpers sit at the very bottom. The reader reads to the depth they need and stops. In the second edition this metaphor, together with the stepdown rule, moved into the "Be Polite" chapter; even the title says what is meant: be polite to the person who will read your file.

The OrderPlacementService we wrote in the functions post has this structure: the only public method, PlaceOrder, at the top, below it the CheckCanOrder, CreateOrder and CompleteOrder it calls, and one-line helpers such as FindBook and ToMinorUnits at the bottom. The legacy OrderService does the opposite: the file preserves the order in which things were written in 2017 and added later; UpdateStatus, ImportBooks and the dead OldCalcDiscount ended up in the middle of the file, SendMail and the cache methods at the very end.

Vertical Layout: Openness, Density and Distance

Martin describes vertical layout with three ideas. Vertical openness: put a blank line between different thoughts. A blank line tells the reader "a new step starts here". The 136 lines of the legacy ProcessData contain not a single blank line; pricing, discount, payment and points are glued together. The comments // discount, // shipping and // payment are trying to stand in for exactly those blank lines.

Vertical density: don't put blank lines between closely related lines. Five assignments that build an object are a paragraph; put blank lines between them and the reader thinks they are separate jobs.

Vertical distance: things that use each other should sit close together. A local variable should be declared right above where it's first used, and a method should sit right below the method that calls it. In the legacy file GetQuote sits right below ProcessData, of which it is a copy, but KargoUcretiHesapla, which both use, comes after it; nothing uses CheckStock, yet it sits next to IsValid. Every time you scroll up and down to find a method in a file, you pay a vertical distance cost.

Member Order and One Type per File

There is no single official rule for member order in .NET, but there is a common consensus. The runtime team's coding style document says fields should be declared at the top of the type. StyleCop (an open-source, Roslyn-based style analyzer package) imposes a more detailed order: fields first, then constructors, properties and methods; within each group public before private; static members before instance members. In practice this order works well:

• Constants and fields.
• Constructors.
• Public properties and methods, the most important first.
• Private helpers, in the order of the public methods that call them (stepdown).

After adding the StyleCop.Analyzers package to the legacy file, the build reported 190 warnings. A few of the formatting-related ones:

OrderService.cs(473,22): warning SA1201: A interface should not follow a class
OrderService.cs(537,25): warning SA1204: Static members should appear before non-static members
OrderService.cs(19,21): warning SA1202: 'public' members should come before 'private' members
OrderService.cs(478,18): warning SA1402: File may only contain a single type
OrderService.cs(573,11): warning SA1403: File may only contain a single namespace

SA1402 (File may only contain a single type) repeated ten times in the file: OrderService.cs holds twelve types, and someone looking for Book can't guess they'll find it in OrderService.cs. "One type per file, file name matches the type name" is almost universal in the .NET world, and IDEs (Integrated Development Environments) offer a "Move type to matching file" refactoring that does it in one click.

20 of those 190 warnings came from SA1101 (Prefix local calls with this): by default StyleCop wants the this. prefix on instance members. The runtime team's style says exactly the opposite: don't use this. unless necessary. Two respected sources contradict each other, and that's a good lesson: most formatting rules aren't right or wrong, they're a choice. What matters is that the team chooses once, writes it into .editorconfig and lets a tool enforce it; which one it chooses comes second.

File-Scoped Namespaces

The file-scoped namespace introduced in C# 10 (namespace Bookstore.Shipping;) pulls every line in the file one indentation level to the left. It looks like a small thing, but 4 spaces is horizontal room on every line, and you feel the difference in a method with nested if blocks. Microsoft's C# coding conventions page also recommends file-scoped namespaces because most files declare a single namespace. You don't have to convert by hand; write csharp_style_namespace_declarations = file_scoped:warning into .editorconfig and run dotnet format style, and the tool does it. Before:

namespace Bookstore.Shipping
{
    public sealed record ShippingLabel(string Name, string Street, string Zip, string City)
    {
        public override string ToString() => $"{Name}\n{Street}\n{Zip} {City}";
    }
}

$ dotnet format style --diagnostics IDE0161 --severity warn

After:

namespace Bookstore.Shipping;

public sealed record ShippingLabel(string Name, string Street, string Zip, string City)
{
    public override string ToString() => $"{Name}\n{Street}\n{Zip} {City}";
}

This conversion doesn't work on the legacy file, because the file has two namespaces (Bookstore.Legacy and Vendor.Payments) and there can be only one file-scoped namespace per file. Sometimes a formatting rule needs a structural fix first: split the types into separate files, then convert the namespace. One more note: IDE rules such as IDE0161 don't run during dotnet build by default; to see them in the build you need to turn on the EnforceCodeStyleInBuild property.

Horizontal Layout: Line Length and LINQ Chains

The lines in the legacy file longer than 120 characters:

$ awk 'length > 120 { print FILENAME ":" NR ": " length }' OrderService.cs
OrderService.cs:140: 135
OrderService.cs:232: 132
OrderService.cs:298: 196
OrderService.cs:324: 160
OrderService.cs:345: 259
OrderService.cs:355: 144
OrderService.cs:546: 144
OrderService.cs:547: 141
OrderService.cs:548: 152
OrderService.cs:549: 166
OrderService.cs:550: 261
OrderService.cs:551: 264
OrderService.cs:552: 267
OrderService.cs:598: 126

Fourteen lines. In the 2008 edition Martin gives his own limit as 120 characters; Microsoft's documentation samples are limited to 65 characters so they stay readable on mobile. There is no single right number, but 259 characters is too much by any measure: it means horizontal scrolling in a code review tool and the whole line showing as "changed" in a diff.

Line 345 is the one-line LINQ (Language Integrated Query) chain inside the Report method:

public string Report(DateTime from, DateTime to, bool csv)
{
    var data = Db.Orders.Where(x => x.Date >= from && x.Date <= to && x.Status != 4 && x.Status != 9).GroupBy(x => x.Customer.Name).Select(g => new { Name = g.Key, Count = g.Count(), Sum = g.Sum(x => x.Total) }).OrderByDescending(x => x.Sum).ToList();
    StringBuilder sb = new StringBuilder();
    if (csv)
    {
        sb.AppendLine("Customer;Orders;Total");
        foreach (var r in data) sb.AppendLine(r.Name + ";" + r.Count + ";" + r.Sum);
    }
    else
    {
        sb.AppendLine("SALES REPORT " + from.ToShortDateString() + " - " + to.ToShortDateString());
        foreach (var r in data) sb.AppendLine(r.Name.PadRight(20) + r.Count.ToString().PadLeft(5) + r.Sum.ToString("0.00").PadLeft(12));
    }
    return sb.ToString();
}

I wrote the same query without changing any name or behavior, only by splitting it into lines; each operator on its own line with the dot at the start of the line:

using System.Text;
using Bookstore.Legacy;

namespace Bookstore.Reporting;

public static class SalesReport
{
    public static string RenderCsv(IEnumerable<Order> orders, DateTime from, DateTime to)
    {
        var data = orders
            .Where(x => x.Date >= from && x.Date <= to && x.Status != 4 && x.Status != 9)
            .GroupBy(x => x.Customer.Name)
            .Select(g => new { Name = g.Key, Count = g.Count(), Sum = g.Sum(x => x.Total) })
            .OrderByDescending(x => x.Sum)
            .ToList();

        var sb = new StringBuilder();
        sb.AppendLine("Customer;Orders;Total");
        foreach (var r in data)
        {
            sb.AppendLine(r.Name + ";" + r.Count + ";" + r.Sum);
        }

        return sb.ToString();
    }
}

Customer;Orders;Total
Ada Lovelace;2;674.39
Alan Turing;1;607.34
Same output: True

The program reported the same orders with the legacy Report and the new RenderCsv and compared the two outputs: identical. The only difference is in the reading. Every step of the chain is now a line: filter, group, project, sort, materialize. In a code review, a diff that changes the Status != 9 condition shows only that line, not the whole 259-character one. Putting the dot at the start of the line (.Where, .GroupBy) shows on the far left that the chain continues; Microsoft's style guide also recommends breaking lines before binary operators, and the same logic applies to the dot in a chain.

Automating with dotnet format

A formatting rule should be enforced by a tool, not a person. dotnet format ships with the SDK (Software Development Kit) and has three subcommands: whitespace (indentation, spacing, line endings), style (the IDE rules in .editorconfig, such as file-scoped namespaces or var usage) and analyzers (code fixes from third-party analyzers). Run with --verify-no-changes, it doesn't touch files, only reports, and returns a non-zero exit code when a change is needed (2 in the example above); that's why it can be a step in a CI (Continuous Integration) pipeline. Designing .editorconfig, tuning analyzer severities and managing them as a team is the subject of the upcoming Coding Conventions and Team Standards series; the message here is just this: the formatting discussion happens once, gets written into a file, and a tool enforces it.

Common Pitfalls

Spending team time on formatting debates. A half-hour code review argument about brace placement or var usage is a topic that one .editorconfig line can close. Martin's "Team Rules" heading in the second edition says the same: the team decides once, and everyone follows it, even if their own preference differs.

Reformatting the whole code base in one commit. Such a commit breaks git blame: every line's last author becomes "the format commit" and the real history is buried. Either format files as you touch them, or do the bulk formatting in a separate commit and add that commit to a .git-blame-ignore-revs file used with git blame --ignore-revs-file; GitHub recognizes this file too.

Sending formatting and a behavior change in the same PR (pull request). A changed condition gets lost among 300 lines whose indentation changed. Format first, separate commit; then the change.

Everyone working with their own IDE settings. When one person uses tabs and another four spaces, unchanged lines get into the diff on every commit. An .editorconfig at the root of the repository removes that difference.

Aligning columns by hand. Adding spaces to line up the = signs of assignments looks tidy at first, but it draws the eye to the alignment instead of the value on the right, and when the longest name changes the whole block enters the diff. Martin also describes in the 2008 edition how he gave up this kind of horizontal alignment.

A Small Experiment

★☆☆ Fix the ConfirmationSender above by using braces for the if, and decide whether the log line belongs inside or outside. Then add csharp_prefer_braces = true:warning to .editorconfig and run dotnet format style; what does the tool do?

★★☆ Split the twelve types in the legacy OrderService.cs into separate files, convert each file to a file-scoped namespace and arrange the members of the OrderService class in the order above. How many warnings are left with StyleCop.Analyzers? Which ones would you turn off on purpose?

★★★ Put the legacy file into a Git repository, first format everything with dotnet format in a single commit, then make a small change in ProcessData. Look at the git blame output; then add the format commit to .git-blame-ignore-revs and run git blame --ignore-revs-file .git-blame-ignore-revs again. Which lines changed owners?

In the next post we move from the file's appearance to the data inside it: an Order full of public fields, the book format and customer type kept as string, chains such as o.Customer.Address.City.Name. We'll talk about the difference between objects and data structures, record for DTOs (Data Transfer Objects) and primitive obsession.