4 ms·
Types can definitely help with data well-formedness but personally I'm very suspicious of 'mini-types' like Name and CartID. If the data you're wrapping has no
by dnomad 8y ago
Types can definitely help with data well-formedness but personally I'm very suspicious of 'mini-types' like Name and CartID. If the data you're wrapping has no internal structure (that can be formally verified) and you're just using the type system as an argument checker then it's not clear to me that you're really adding much value. If CartID and ProductID are both just strings what's to stop a user from doing the same as:
CartID cartId = new CartID(productId.toString());
addToCart(..., cartId, ...); <-- but look that's really a ProductID!
I'd suggest that while you may have reduced the likelihood of certain errors you have not actually eliminated those errors.
The right approach here is to be very suspicious of any methods in your domain that just take a bunch of IDs. Either:
(a) Force the caller to resolve the IDs to actual entities before invoking the method:
addToCart(Cart c, Product p, ...)
or (b) Represent the method parameters as a Parameters object or, preferrably, an Event object.
handle(ProductAddedEvent event);
The real problem here is not the type system but a domain that is not sufficiently protected by an Anti-Corruption Layer [1]. Unfortunately most developers are not familiar with this kind of strategic design and languages are not much help here. It would be interesting if there was a language that could flag things like weak ACLs.
[1] https://docs.microsoft.com/en-us/azure/architecture/patterns/anti-corruption-layer https://docs.microsoft.com/en-us/azure/architecture/patterns...
- gcommer 8y ago> 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.
- dnomad 8y agoAs 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.
- catern 8y ago>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)
- jpitz 8y ago
- raquo 8y agoYou 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.
- gizmo686 8y agoThis 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.
- iovrthoughtthis 8y agoThis. 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.
- catern 8y agoI'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.
- sgt101 8y agoI 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.
- pseudonom- 8y agoIn 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.
- sbov 8y ago> (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.
- 0x445442 8y agoI 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.
- chriswarbo 8y agoI 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.