Hacker News Reader

This line of code is very suspicious:

    CartID cartId = new CartID(productId.toString());

This line of code looks perfectly fine:

    addToCart(..., productId, ...);

There is tremendous benefit to making wrong code obviously wrong.

Not to mention that even if you do not have formally verifiable structure, you can almost always do sanity checks.

You can get very far by just containing unsafety. In this example, you shouldn't need to sprinkle `new CardID` calls throughout your whole application. You will only use them when deserializing values from a typeless source (e.g. AJAX response), and that part should be contained to some AjaxService.

I think that this is a smoking gun where the user has purposefully violated the typing that's been introduced to stop errors. It's like altering tests so your code passes - basically sabotage.

> new CartID(productID.toString());

With good data modeling and safety checks, this should fail: a product id and cart id should have some sort of distinguishing syntax (eg "P1234" vs "C1234") and so `new CartID("P1234")` will fail.

At some point, no sorts of type system can stop incorrectly implemented/specified logic. An even simpler example would be just passing some constant, valid, but not semantically correct string to the CartID constructor.

Your (a) and (b) solutions don't solve this. I could just as easily create a meaningless Cart object, or a meaningless ProductAddedEvent the same way you constructed that meaningless CartID.

As I said, if your data actually has internal structure that can be verified then the Type is meaningful. (Though that sort of prefixing introduces its own problems and I think most IDs just end up being UUIDs or URIs or numbers.) Otherwise you're just wrapping strings. The point here is that a Type represents data and behavior or, more formally, Types represent constraints over a space of data and behavior. Types that don't actually do any constraining are useless.

> I could just as easily create a meaningless Cart object, or a meaningless ProductAddedEvent the same way you constructed that meaningless CartID.

The problem here is not mixing up IDs. The problem is actually guaranteeing that the pre-conditions of addToCart() have been satisfied. Here, for example, addToCart() probably wants more than just a Cart and Product object, but a Cart and Product objects that have been persisted. Presumably the domain has been constrained so that you can't create meaningless objects. Otherwise what's the point of any object? If the only valid Cart and Product objects are those that can be retrieved from a persistent repository then, yes, this not only solves the trivial problem of mixing up IDs, but also the more valuable underlying problem that addToCart() is constrained to act on real, valid (according to the domain) Cart and Product objects.

In other words, this isn't really an issue of language design. And while there's a tendency to believe the language can solve every problems and that design patterns are a sign of language "weakness" the reality is developers that really want to solve these problems will have to think carefully about their domain and apply the appropriate design patterns.

>Types that don't actually do any constraining are useless.

Exactly!

I can make CartId/ProductId/WhateverId types that are just wrappers around String. But that's not actually using the type system to enforce correct behavior. Nothing in the type system prevents me from violating the invariants of my application.

I would only actually be using the type system, if my types were actually constrained so that only valid CartIds, that refer to real Carts, could be created. Then I would actually have a useful invariant, which the type system guarantees cannot be violated. (inasmuch as the validation step when creating CartIds is correct)

But aren't you then constrained until the end of time to ensure that cart ids and product ids are entirely disjoint? That's way too coupled for my little brain.

In some languages, you can ensure that no `.toString()` equivalent is available. Then, your ID is truly opaque and can only be used in the prescribed ways.

> (a) Force the caller to resolve the IDs to actual entities before invoking the method:

This doesn't really fix the potential problem you pointed out. All it turns it into is this:

> Cart cart = lookupCart(productId);

With the popularity of auto increment, that above code will likely return a valid cart.

I came to say the same thing. I appreciated the blog post but the example was overly contrived. The addToCart method should have been passed the objects the ids were representing and should not have been responsible (even via delegation) for resolving the ids.

I would say the unsafe thing here is `new CartID`, which allows you to turn an arbitrary string into a `CartID`. That shouldn't be possible outside of a small, encapsulated module somewhere, which exposes only safe usages (e.g. wrappers around database calls).

One way to do this is via a module system, like in Racket, ML, Haskell, etc. I'm not sure if Java et al. allow private constructors, but if so we could use those and provide the safe wrappers as static methods.

An alternative/complementary approach is to make `CartID` abstract, e.g. an existential type in ML or an interface in Java. This way our code is forced to be generic, working for any possible implementation of `CartID`. We specialise it to some sort of `StringCartID` implementation once, at the top-level.

This. Doing actions against an id is a shortcut. Deserialise the object from the db so you can make changes and verify integrity before serialization.

Also allows you to leverage the type of object your playing with.

No need for CartId types if your passing around Cart types. Which you probably already have.

I'd go further than saying "doing actions against an id is a shortcut". I say it's an antipattern. Ids should only ever exist at the edge of your system: When you're taking serialized data in and turning it into objects, or you're taking objects and turning them into serialized data out. That serialization should be handled completely separately from your application logic; you shouldn't see (non-opaque) ids in your application logic.