Comments in Clean Code: When to Write Them and When to Turn Them into Code

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

Guess First

These are all the comments in the legacy OrderService.cs, except for commented-out code, with their line numbers:

   1  // OrderService.cs
   2  // Bookstore order module
   3  // Written in 2017, everyone has added something since
  26  // Processes the order
  27  // 03.05.2019 A.K. - added coupons
  28  // 17.11.2020 M.T. - added express shipping
  29  // 09.02.2022 A.K. - added VIP discount
  30  // 21.06.2023 S.Y. - added weekend discount
  71  // calculate the price
  81  // discount
 102  // shipping
 104  if (flag) k += 12.5m; // gift wrap
 107  o.Total = Math.Round((tmp - tmp2) * 1.1m + k, 2); // 10% tax
 108  // payment
 126  // decrease stock
 131  // add points
 168  // price quote (for the cart page)
 248  // the customer's orders
 256  // is the order valid?
 290  // hand over to shipping
 323  // create a customer
 360  // invoice
 384  // update the status
 406  // load books from CSV (isbn;title;author;price;stock;type)
 438  // old discount calculation - not used but do not delete!
 472  // for different order types in the future
 483  public int Status; // 0 new, 1 paid, 2 approved, 3 shipped, 4 cancelled, 9 payment failed
 575  // Simulates the payment provider's SDK (third-party code)

Sort this list into three groups: keep as is, delete, and turn into code. And ask one more question: which of these comments are lying? Hint: a comment doesn't have to say something wrong to be wrong; leaving something out is enough.

What Code Can't Say and a Comment Should

Here is how I sorted them. The only one that stays is the comment on line 575: it says that PaymentClient is an imitation, not a real SDK (Software Development Kit), and there is no way to infer that from the code. The CSV (Comma-Separated Values) format note on line 406 also carries valuable information, but its place is not a loose line above the method; it belongs in an XML (Extensible Markup Language) doc comment, which we'll see below, or in a type that defines the format.

The delete group is crowded. Lines 26, 71, 81, 102, 108, 126, 131, 168, 248, 323, 360 and 384 repeat what the code already says: right below // shipping, KargoUcretiHesapla is called. The dated lines 27–30 are a changelog; Git already keeps that, with author and date. The "not used but do not delete!" warning on line 438 protects dead code and doesn't say why it shouldn't be deleted. Line 472 defends an unused generic interface with "we'll need it in the future".

The turn-into-code group is the most instructive. // gift wrap on line 104 exists because the parameter is called flag. // 10% tax on line 107 exists because 1.1m carries no name. // 0 new, 1 paid, ... on line 483 exists because Status is an int. In all three, the comment compensates for what the code can't express. The Comments chapter of the second edition opens with a heading that says exactly that, "Compensating for Failure": for Robert C. Martin (author of Clean Code), a comment is a compensation we reach for where we failed to express intent in code. If you can remove the need for the compensation, fix the code, not the comment.

The liars are 256 and 290. // is the order valid? says IsValid asks a question, but the method sets the order's status to 2. // hand over to shipping doesn't say that ShipOrder writes the shipping label to disk, changes the status and sends the customer an e-mail. The code changed, the comment didn't. That is the real reason Martin is wary of comments: the compiler doesn't read them, tests don't verify them, refactoring tools don't update them.

Bad Comments and What Git Already Keeps

Martin lists bad comments at length; the legacy file contains almost the entire list. The ones I see most often:

The comment that repeats the code. // decrease stock and the oi.Book.Stock = oi.Book.Stock - oi.Qty; below it give the same information twice. It gains the reader nothing but adds maintenance cost: when the code changes, the comment has to change too.

The changelog comment. // 17.11.2020 M.T. - added express shipping tells you who added what and when, but not which lines were added, why, or what has changed since. git log and git blame tell you all of that; we covered these commands in detail in the Git series post "Looking at Git History: git log, git show, git blame and Aliases".

Commented-out code. The four commented-out lines of ProcessDataV2 and the dead OldCalcDiscount method are kept because "we might need them one day". Nobody dares delete them, because nobody knows why they are there. SonarAnalyzer (SonarSource's Roslyn analyzer package for C#) catches them on the first build:

OrderService.cs(439,25): warning S1144: Remove the unused private method 'OldCalcDiscount'.
OrderService.cs(446,9): warning S125: Remove this commented out code.

The fear of deleting is a fear of losing, but deleted code doesn't get lost in Git. I put the legacy file into a repository, deleted the dead code in a second commit, then searched for it again:

$ git log -S "OldCalcDiscount" --oneline
015f737 Remove dead code: OldCalcDiscount and commented-out ProcessDataV2
3558512 Import legacy OrderService
$ git show HEAD~1:OrderService.cs | grep -n -A4 "OldCalcDiscount"
439:        private decimal OldCalcDiscount(decimal total, int itemCount)
440-        {
441-            if (itemCount > 3) return total * 0.07m;
442-            return 0;
443-        }

git log -S (the pickaxe) finds the commits in which a string was added or removed. The deleted method came back with two commands, line numbers included. The commit message is also a better place than a comment: the answer to "why was it deleted" sits right there.

Turning Comments into Names and Code

Let's turn three of the compensating comments into code. int Status and the comment explaining it become an enum; 1.1m and // 10% tax become a const:

namespace Bookstore.Ordering;

public enum OrderStatus
{
    New = 0,
    Paid = 1,
    Approved = 2,
    Shipped = 3,
    Cancelled = 4,
    // Values are stored as int in the database; 5-8 are unused because of old records, 9 must not change
    PaymentFailed = 9
}

public static class Pricing
{
    private const decimal TaxRate = 0.10m;

    // The accounting software rounds half kuruş to the nearest even digit; we use the same rule on
    // purpose so the invoice and the charged amount match exactly (FIN-112).
    public static decimal TotalWithTax(decimal discountedSubtotal) =>
        Math.Round(discountedSubtotal * (1 + TaxRate), 2, MidpointRounding.ToEven);

    // TODO(#482): Pick TaxRate by product type once books get their own tax rate
    public static decimal TaxOf(decimal discountedSubtotal) => discountedSubtotal * TaxRate;
}

690.52
PaymentFailed = 9

OrderStatus.PaymentFailed now explains itself, and you don't have to remember 9. The name TaxRate replaced // 10% tax. But notice: the comments didn't disappear entirely, they moved. The comment inside the enum says why the values jump from 4 to 9; that can't be inferred from the code, and if someone "tidies up" PaymentFailed to 5, the meaning of old records in the database changes.

The same goes for // gift wrap: in the previous post we turned the bool flag parameter into the GiftWrap property of the PlaceOrderRequest record, and the comment was no longer needed. // calculate the price wants to be a method name: UnitPriceOf(book). The rule is simple: if a comment says "what" something is, that "what" can be a name.

Good Comments: Why, Warnings and Context

The comment above TotalWithTax is one of the most valuable lines in this post. The code already says it uses MidpointRounding.ToEven; the comment says why: the accounting software uses the same rule and the invoice must match the amount charged exactly. (The reason was made up for this example; in a real project you write whatever the real reason is.) This information can't be turned into code. Someone who writes AwayFromZero instead of ToEven gets no compile error, and the tests may not catch a one-kuruş difference; the only thing that will stop them is these two lines.

What good comments have in common is that they carry information the code can't:

• Why: the reason for a business rule, a regulation or a decision that looks odd.
• Warning: "this method is not idempotent, don't retry it", "this order matters, don't change it".
• External context: a workaround for a bug and its link, a clause of an RFC (Request for Comments), a ticket number.
• Intent and contract: what a public API (Application Programming Interface) guarantees and which exceptions it throws.

A TODO comment can join this list, but only when it is trackable. // TODO: fix is a wish; // TODO(#482): ... is a piece of work with an owner and a priority in the issue tracker. With SonarAnalyzer's S1135 (TODO) rule enabled, the same file produced:

Pricing.cs(23,8): warning S1135: Complete the task associated to this 'TODO' comment.

You may not want this warning on every TODO; but a team rule like "no TODO without an issue number gets merged" prevents wishes that live in the code for years.

XML Doc Comments for Public APIs

One kind of comment is different from the others: XML doc comments starting with ///. They show up in IntelliSense, go into the NuGet package as an XML file and, most importantly, the compiler partly verifies them. When you set the GenerateDocumentationFile property to true, the compiler produces the XML file and checks the doc comments. A file with four deliberate mistakes:

namespace Bookstore.Ordering;

/// <summary>An order request.</summary>
/// <param name="CustomerId">The number of the customer placing the order.</param>
/// <param name="QuantitiesByIsbn">Quantities by ISBN.</param>
/// <param name="CouponCode">Coupon code; <see langword="null"/> if there is none.</param>
public sealed record PlaceOrderRequest(int CustomerId, IReadOnlyDictionary<string, int> QuantitiesByIsbn, string? CouponCode);

/// <summary>Thrown when an order is rejected by a business rule.</summary>
public sealed class OrderRejectedException(string reason) : Exception(reason);

/// <summary>Creates orders for registered customers and takes the payment.</summary>
public sealed class OrderPlacementService
{
    /// <summary>Creates the order and charges the customer's saved card.</summary>
    /// <param name="req">Customer, cart and coupon.</param>
    /// <returns>The new order's number.</returns>
    /// <exception cref="ArgumentNullException"><paramref name="request"/> is null.</exception>
    /// <exception cref="OrderRejectedExcepton">The cart is empty or the coupon is unknown.</exception>
    public int PlaceOrder(PlaceOrderRequest request)
    {
        ArgumentNullException.ThrowIfNull(request);
        if (request.QuantitiesByIsbn.Count == 0) throw new OrderRejectedException("Empty cart");
        return 1000;
    }

    public decimal GetQuote(PlaceOrderRequest request) => 0m;
}

OrderPlacementService.cs(16,22): warning CS1572: XML comment has a param tag for 'req', but there is no parameter by that name
OrderPlacementService.cs(19,26): warning CS1574: XML comment has cref attribute 'OrderRejectedExcepton' that could not be resolved
OrderPlacementService.cs(20,45): warning CS1573: Parameter 'request' has no matching param tag in the XML comment for 'OrderPlacementService.PlaceOrder(PlaceOrderRequest)' (but other parameters do)
OrderPlacementService.cs(27,20): warning CS1591: Missing XML comment for publicly visible type or member 'OrderPlacementService.GetQuote(PlaceOrderRequest)'

When a parameter is renamed and its doc comment isn't (CS1572, CS1573), or the exception class name is misspelled (CS1574), the build warns you. An ordinary comment has no such guarantee. The fixed PlaceOrder:

/// <summary>Creates the order and charges the customer's saved card.</summary>
/// <param name="request">Customer, cart and coupon.</param>
/// <returns>The new order's number.</returns>
/// <exception cref="ArgumentNullException"><paramref name="request"/> is null.</exception>
/// <exception cref="OrderRejectedException">The cart is empty or the coupon is unknown.</exception>
public int PlaceOrder(PlaceOrderRequest request)

In the generated XML file each member sits under an ID such as M:Bookstore.Ordering.OrderPlacementService.PlaceOrder(Bookstore.Ordering.PlaceOrderRequest), and crefs are resolved to full type names such as T:Bookstore.Ordering.OrderRejectedException; documentation tools read this file.

The same rule applies to the content of a doc comment: don't repeat the code. A summary like /// <summary>Gets the quote.</summary> adds nothing. What is valuable is the information you can't get from the signature: that GetQuote creates no order and charges nothing, which exception PlaceOrder throws in which situation, which unit a value is in. CS1591 asks for docs on every public member and can turn into noise in application code (such as the controllers of a Web API); teams usually keep this warning on only in library projects published as packages.

The Opposing View: Ousterhout and Interface Comments

John Ousterhout (Stanford professor, author of A Philosophy of Software Design) is far more positive about comments than Martin. For him a comment is part of the abstraction: a good interface comment tells callers everything they need to know without reading the implementation; constraints, side effects, why it exists. Code tells "what" and "how", but the "why" and the "what do I guarantee" in the designer's head don't come out of the code. That's why Ousterhout opposes the idea of replacing comments with long method names.

In their 2024–2025 exchange with Martin (included in the second edition as an appendix titled "The Clean Code Debate") the two didn't fully agree, but their common ground was wider than expected: public APIs need comments, some internal interfaces do too; non-obvious code should be explained; comments should be reviewed like code and fixed when they're wrong. Where they part is emphasis: Martin sees a comment as a last resort and wants the code fixed first, Ousterhout sees it as the first step of design and even recommends writing the interface comment before the method.

In practice the rule that combines them is: say in code what code can say, and write in a comment what it can't. Turning int Status into an enum is Martin's lesson; writing down why ToEven was chosen is Ousterhout's.

Common Pitfalls

Not updating a comment together with the code. The most dangerous comment is one that used to be true. In code review, read the comment above the changed code too: is it still true?

Making a summary on every method a rule. "Every public method must have a doc comment" produces lines like /// <summary>Saves the order.</summary> that repeat the name SaveOrder. The rule should be "every public method that can't be understood without one".

Leaving commented-out code "temporarily". Temporary is as permanent as anything in Git. Delete the code and write why in the commit message.

Covering a bad name with a comment. int elapsedDays; instead of int d; // days elapsed. The moment you feel the need to explain a variable next to it, it's time to rename it.

Closing-brace comments and divider lines. } // end foreach or // ======== HELPERS ======== are signs that the method is too long; the fix is not a comment but splitting the method.

Hiding a big class behind #region. Folding regions in Visual Studio make a 462-line class look like 30 lines but don't make it smaller. The need to split a class into regions tells you it has more than one responsibility.

A Small Experiment

★☆☆ Delete the comments on lines 26, 71, 81, 102, 108, 126 and 131 of the legacy OrderService.cs. Read ProcessData again: what information did you lose? If you lost any, move it into a method name or a variable name.

★★☆ Turn on GenerateDocumentationFile in a project and add <WarningsAsErrors>CS1572;CS1573;CS1574</WarningsAsErrors>. Then rename a public method's parameter by hand, not with the IDE (Integrated Development Environment); what does the build say?

★★★ Put the legacy file into a Git repository, delete the commented-out ProcessDataV2 and OldCalcDiscount in separate commits, and find both again with git log -S. Then follow the history of just ProcessData's line range with git log -L 31,166:OrderService.cs and compare it with what the dated changelog comments give you.

In the next post we move on to the visual layout of this file: twelve types in one file, members in random order and a 259-character one-line LINQ (Language Integrated Query) chain. We'll look at the newspaper metaphor, member order, how indentation can lie, and which part of this work dotnet format can do for you.