Title and Text, Swapped Since Commit One

The most expensive class of bug is not the one that crashes. It is the one that compiles, runs, produces plausible-looking output, and is invisible because the code that would expose it never executes. Part 2 showed StoryText carrying an implicit conversion from string over an unvalidated constructor. This part is the bill for it: a document-to-entity mapper in Trill's Stories service that has had two constructor arguments transposed since the repository's first commit, and a dead read path that has hidden it ever since.

The mapper

Trill.Services.Stories.Infrastructure/Mongo/Documents/StoryDocument.cs, lines 38 to 40:

public Story ToEntity()
    => new Story(Id, Author.ToValueObject(), Text, Title, Tags,
        GetDate(CreatedAt), new Visibility(GetDate(From), GetDate(To), Highlighted));

And the constructor it is calling, Core/Entities/Story.cs:26:

public Story(StoryId id, Author author, string title, StoryText text, IEnumerable<string> tags,
    DateTime createdAt, Visibility visibility = null, int version = 0)

Line them up positionally and the third and fourth arguments are the wrong way round. The document's Text — the body of the story, up to 200 characters — is passed into the parameter named title. The document's Title is passed into the parameter named text. Every Story entity this method produces has its title and its body exchanged.

On the same class, twenty lines up, the write direction is correct:

public StoryDocument(Story story)
{
    Id = story.Id;
    Author = new AuthorDocument(story.Author);
    Title = story.Title;
    Text = story.Text;
    ...
}

StoryDocument(Story) maps by name, member to member, and cannot get it wrong. ToEntity() maps by position into a constructor, and did.

Why the compiler let it through

StoryDocument.Title and StoryDocument.Text are both string. The constructor's title parameter is string and its text parameter is StoryText. So the swap should have been a type error: you cannot pass a string where a StoryText is required.

Except you can, because of this one line in Core/ValueObjects/StoryText.cs:

public static implicit operator StoryText(string storyText) => new StoryText(storyText);

• Text into string title is a plain string-to-string assignment. No conversion, no complaint.
• Title into StoryText text triggers the implicit operator, which wraps the string in a StoryText and returns it. No cast in the source, no warning, no diagnostic.

One user-defined conversion operator turned a type error into a silent semantic error. That is precisely what implicit conversions are for — removing ceremony — and precisely their cost: they remove the type system's ability to distinguish two strings that mean different things. A StoryTitle value object with no implicit conversion, or C# named arguments at this call site, or a positional record with by-name construction would each have caught it at compile time. None of the three is present.

Nullable reference types would not have helped, and they are switched off anyway — no .csproj in either service sets <Nullable>, <LangVersion> or <TreatWarningsAsErrors>, and there is no .editorconfig and no analyzer package anywhere in the estate.

The sibling mapper that got away with it

UserDocument, in the same folder, maps positionally too:

public User ToEntity() => new(Id, Name, CreatedAt, Rating, Locked);

Five arguments against User(UserId id, string name, DateTime createdAt, int rating = 0, bool locked = false, int version = 0). Same technique, same hazard — and it is correct, because no two adjacent parameters share a type. Guid, string, DateTime, int, bool: any transposition is a compile error. StoryRatingDocument.ToEntity() is safe for the same reason.

So the difference between the correct mapper and the broken one is not care; it is the coincidence of type distinctness. Three mappers were written the same way by the same author in the same commit, and the one where two neighbouring parameters could be confused is the one that was. That is the argument for by-name construction stated as an experiment with a control group.

It is also worth picturing what the corrupted entity looks like at runtime, because it is not subtle. A Story loaded through ToEntity() has a Title of up to 200 characters — the whole body — and a Text holding the title. Story's constructor validates the title with string.IsNullOrWhiteSpace, which a 200-character body passes, and validates the text not at all, because as part 2 showed, StoryText's constructor has no rules in it. Both guards that could have noticed are the guards that were never armed.

Since commit one

git log on that single file, across every branch in the repository, returns exactly one commit:

5b23614 init

The file has never been edited. And checking the init version of Story.cs confirms the constructor's parameter order was identical then — StoryId id, Author author, string title, StoryText text, … — so the swap was not introduced by a later signature change. It shipped in the first commit, survived a decorators commit and a large domain refactor, and is still there at the last commit of the repository.

Why it has never bitten

This is the part that makes the bug worth a whole article rather than a bug report. The mapper is wrong; the system does not visibly misbehave. Tracing every call site explains why.

ToEntity() has exactly one caller, Mongo/Repositories/StoryMongoRepository.cs:19:

public async Task<Story> GetAsync(StoryId id)
{
    var document = await _repository.GetAsync(r => r.Id == id);
    return document?.ToEntity();
}

IStoryRepository.GetAsync in turn has exactly two callers:

• RateStoryHandler.cs:40. It fetches the story, null-checks it, and then uses story.Id and nothing else. Neither Title nor Text is read.
• StoryRatingChangedHandler.cs:20. It fetches the story to find the author, then credits their reputation. This handler is unreachable — the domain event that would trigger it is never raised, which is part 4's subject.

And the interface itself closes the loop:

public interface IStoryRepository
{
    Task<Story> GetAsync(StoryId id);
    Task AddAsync(Story story);
}

There is no UpdateAsync. A Story entity, once loaded, can never be written back. So even the corrupted objects that ToEntity() does produce cannot persist their corruption to Mongo.

Meanwhile the entire read side never goes near the entity at all. BrowseStoriesHandler and GetStoryHandler live in Infrastructure/Mongo/Queries/Handlers, take IMongoDatabase directly, and project StoryDocument straight to a DTO via Map<T> — Title = story.Title, by name. Users see correct titles and correct bodies on every screen, because the screens are fed by a code path that skips the broken mapper entirely.

Three independent accidents therefore keep the defect latent: the only live caller ignores the swapped fields, the only caller that would read them is dead code, and the repository has no write-back method. Remove any one of them and the bug becomes a data-corruption incident on the next deploy.

What this changes about how I read mapping code

I have written the phrase “it's just a mapper” in a review. I will not again.

• Positional construction across a semantic boundary is a hazard, not a style choice. Document-to-entity, DTO-to-command, request-to-model — these are exactly the places where two adjacent parameters have the same primitive type and different meanings. Use named arguments, use object initialisers, use record with with, use anything that binds by name. StoryDocument's forward mapper does bind by name and is correct; the reverse one does not and is not.
• An implicit conversion operator is a permanent, project-wide waiver of a type check. It is worth it for a genuine widening (int to Rate, where the constructor validates). It is rarely worth it for string to anything, because string is the type every other string can be mistaken for.
• Dead code is not free; it is camouflage. The reason nobody noticed is that the observation point was disabled. Coverage tooling would have shown StoryRatingChangedHandler at zero, and zero coverage on a handler is a question, not a statistic.
• One test would have found it. new StoryDocument(story).ToEntity() asserted equal to story on Title and Text is a four-line round-trip test. The repository has four test projects and, as part 15 documents, zero test files.

The durable rule, stated plainly: if two parameters of the same underlying type sit next to each other in a constructor, the type system has already stopped protecting you, and only naming can. Trill's authors reached for a value object to solve exactly that problem, then attached an implicit conversion that undid it.

Next, the refactor that left StoryRatingChangedHandler unreachable in the first place — the refactor that stopped halfway.