The Factory That Doesn't Own Construction

A factory that validates is only a factory if it is the only way in. Leave the constructor public and you have not built a factory; you have built a helper method with an aspirational name, and every future reader who sees IStoryTextFactory in a constructor parameter list will assume an invariant that does not hold. Part 1 established that Trill's Stories service has a domain assembly with no package references — a genuinely disciplined shape. This part reads the one value object in that assembly where the discipline stops at the door.

The factory

Trill.Services.Stories.Core/Factories/StoryTextFactory.cs is the whole thing:

public class StoryTextFactory : IStoryTextFactory
{
    public StoryText Create(string text)
    {
        if (text is null)
        {
            throw new InvalidStoryTextException();
        }

        if (string.IsNullOrWhiteSpace(text))
        {
            throw new EmptyStoryTextException();
        }

        if (text.Length < 10)
        {
            throw new TooShortStoryTextException(text);
        }

        if (text.Length > 200)
        {
            throw new TooLongStoryTextException($"{text.Substring(200)}...");
        }

        return new StoryText(text.Trim());
    }
}

Read it as a rules table and it is good. A story's text is 10 to 200 characters, non-null, non-blank, trimmed. Four distinct exception types, one per rule, which — because of the estate's type-name-to-wire-code convention — surface to the client as invalid_story_text, empty_story_text, too_short_story_text and too_long_story_text. That is a better error taxonomy than most production APIs manage.

Then read line 27 again. text.Substring(200) returns everything from index 200 onward, not the first 200 characters. For a 250-character submission, the error message quotes the last 50 characters of what the user typed and calls them “too long”. The intent was Substring(0, 200). It never throws, because the branch is already guarded by text.Length > 200, so it is a permanently, quietly wrong message with no failure to announce it.

The door beside the door

Here is Core/ValueObjects/StoryText.cs, trimmed to the parts that matter:

public class StoryText : IEquatable<StoryText>
{
    public string Value { get; }

    public StoryText(string value)
    {
        Value = value?.Trim();
    }

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

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

• The constructor is public and validates nothing. new StoryText("x") is a legal one-character story text. new StoryText(null) is a legal StoryText whose Value is null, and GetHashCode even has a branch for it.
• implicit operator StoryText(string) means you do not have to type new to get there. StoryText text = "x"; compiles. So does passing a bare string to any method that takes a StoryText.
• The reverse conversion exists too, so a StoryText silently becomes a string wherever one is expected — which is how StoryDocument's Text property gets populated without a cast.

Put those two files side by side and the shape is clear: the factory owns the rules and the value object owns construction, and nothing connects them. SendStoryHandler does the right thing — var text = _storyTextFactory.Create(command.Text); — and it is the only caller in the service that does. Every other route into a StoryText bypasses all four rules, including the one that matters most in part 3.

Where the factory came from

Git makes the intent legible. The Stories repository has three commits on master, and the last one — 4d96ab8, labelled net5.0 update — is not a framework bump at all. Among many other things it did this:

D  src/…Core/Policies/IStoryTextPolicy.cs
D  src/…Core/Policies/DefaultStoryTextPolicy.cs
A  src/…Core/Factories/IStoryTextFactory.cs
A  src/…Core/Factories/StoryTextFactory.cs

The predecessor was a Strategy, not a Factory:

public interface IStoryTextPolicy
{
    void Verify(StoryText storyText);
}

DefaultStoryTextPolicy.Verify took an already-constructed StoryText and threw the same four exceptions on its Value. So the refactor changed the direction of the relationship — from “validate this object” to “produce a valid object” — which is exactly the move the pattern books recommend when construction and validation belong together. It is the right instinct. It simply stopped one line short of the change that makes the instinct hold: nobody narrowed StoryText's constructor.

The refactor also carried the bug across. DefaultStoryTextPolicy had storyText.Value.Substring(200) in the same position. A rename-and-move refactor moves defects at exactly the same fidelity as it moves features, and the compiler has no opinion about either.

The value object next door gets it right

The instructive part is that the same folder contains a value object which does it properly. Core/ValueObjects/Rate.cs:

public Rate(int value)
{
    if (value is < -1 or > 1)
    {
        throw new InvalidRateException(value);
    }

    Value = value;
}

public static implicit operator Rate(int rate) => new Rate(rate);

Same author, same commit, same implicit-conversion idiom — but the rule lives in the constructor, so the conversion is safe. Rate r = 7; throws. StoryText t = "x"; does not. The implicit operator is not the problem; an implicit operator over an unguarded constructor is. And note the C# 9 relational pattern in that guard: these authors were not unaware of the language. Visibility protects itself the same way, rejecting from >= to in its constructor.

Author picks a third position, and it is the one I would actually recommend:

public Author(Guid id, string name)
{
    if (string.IsNullOrWhiteSpace(name))
    {
        throw new MissingAuthorNameException();
    }

    Id = id;
    Name = name;
}

public static Author Create(User user) => Create(user.Id, user.Name);

public static Author Create(Guid id, string name) => new Author(id, name);

A validating constructor and a named factory method over it. The factory exists to express intent — Author.Create(user) says “an author is derived from a user” in a way new Author(user.Id, user.Name) does not — and it adds no safety because the constructor already has it all. That is the correct division of labour: the constructor owns the invariant, the factory owns the vocabulary. SendStoryHandler calls Author.Create(user) one line before it calls _storyTextFactory.Create(command.Text), so the two idioms sit adjacent in the same method, and only one of them is load-bearing.

So the four value objects in Core/ValueObjects land in three different places on the same spectrum: Rate and Visibility are self-protecting, Author is self-protecting with a convenience factory, and StoryText has its rules living in a different folder behind an interface that anyone can decline to use.

The rules do reach the client, at least

One thing the factory does get unambiguously right is the shape of its failures, and it is worth pausing on because it is the estate's best small idea. Each rule throws its own type — InvalidStoryTextException, EmptyStoryTextException, TooShortStoryTextException, TooLongStoryTextException — and Infrastructure/Exceptions/Extensions.cs turns the type name into the wire code:

public static string GetExceptionCode(this Exception exception)
    => exception.GetType().Name.Underscore().Replace("_exception", string.Empty);

TooShortStoryTextException becomes too_short_story_text. The client receives 400 { "code": "too_short_story_text", "reason": "Too short story text: 'abc'." } and can branch on a stable machine-readable token without anyone maintaining an enum, a resource file or a mapping table. Adding a rule is one class.

It also means the Substring(200) bug is user-visible. reason carries the message the factory built, so a caller who submits 250 characters is shown the last 50 of them, quoted back as though they were the problem. And because the type name is the contract, renaming any of those four classes is a breaking API change with no compiler warning and — as part 15 establishes — no test.

What I would change, and what I would not

The minimal fix is two edits and one deletion:

1. Move the four rules into StoryText's constructor, exactly as Rate does.
2. Delete implicit operator StoryText(string). Keep the outbound conversion if you like — going from a validated object to its string is lossless and harmless.
3. Delete IStoryTextFactory and StoryTextFactory, and drop the dependency from SendStoryHandler, which currently takes ten constructor parameters and would then take nine.

To be fair to the design as written: there is a real case for a factory over a value-object constructor, and it is when validation needs something the value object should not have — a clock, a configuration value, a lookup. If Trill's text limits were per-tier rather than the literals 10 and 200, IStoryTextFactory would be the correct seam and StoryText's constructor would still need to be internal. The pattern is not wrong here; it is unfinished, in the same way that part 4's domain-event pipeline is unfinished. The 4d96ab8 commit is full of good instincts that stop one file early.

The rule I take from this file: a factory earns its name only when the constructor it replaces is unreachable from outside the type. If new still works, you have written a validator and mislabelled it, and the mislabelling is worse than no factory at all — because the next reader trusts the name.

There is one more consequence, and it is the reason this part comes before the next. An implicit conversion from string does not only let unvalidated text in; it removes the compiler's ability to tell a title from a body. Next, title and text, swapped since commit one.