These are great. I’d also add: not everything with 2 states is a bool — give them their own clear enum. Even worse is shoehorning a nullable bool for tri state.
Especially once you have two or more bool parameters to a method.
Imagine if there were a FileStream constructor like so:
Now you’d commonly call that like var fs = new FileStream(“foo.txt”, true, false, false). Or was it false, true, false?
(Named arguments mitigate this, but still.)
The problem with this is that the example is so trivial you could probably refactor it into almost any pattern and make the argument that its clean.
What if the Social Security Number needs to be various formats for different countries? Is the Country of the SSN dependent on the Country in the Address of the user? Do all SSN have the same utility functions?
How about different country addresses? this structure wont work for every country, if you (or other developers) built a lot of application code based off these structural assumptions you’re then going to have to redo _all_ of that code or develop complicated branching or adapters for all the old code.
The problem is you’ve built these structures, abstractions, and restrictions based on superficial patterns that emerged while the program was incomplete. You’ve made assumptions about the future and imposed “organization” on an incomplete subset of the actual problem.
The intent is to be organized, but you’re superficially simplifying the project today, by imposing huge restrictions on the project in the future.
What if the Social Security Number needs to be various formats for different countries? Is the Country of the SSN dependent on the Country in the Address of the user? Do all SSN have the same utility functions?
Doesn’t that make the case even more so that it shouldn’t be a string, and especially not an int?
(But yes, the proposed class has major localization issues. Still, only the user-facing read-only representation — to generate the mailing address to print on a label, say — needs to be a combined string.)
Have to admit I always find it amusing when people write code like that with a “country” field, but then make that field worthless by having things like ZipCode and SSN fields. Bonus points if zipcode is an int (like here), or Zip/SSN/State is validated, since then you can’t even try to re-use those fields to represent an address in another country.
Another benefit which I didn’t see (though apologies if I glossed over it) is by moving properties into classes representing the domain, you avoid accidental comparison and assignment between completely unrelated items and the compiler can catch that. For example, given the sample class:
It becomes possible to make a typo like:
Sure, it’s kind of boneheaded mistake by the developer, but the compiler and type system do nothing to catch it. Once you refactor it to have a public SocialSecurity SocialSecurity { get; set; } property, then that same assignment gets caught by the type system and compiler.
Same goes for the various name, address, email properties. You could straight up make CustomerName, Address, and Email classes and prevent any and all assignment between them.
Is it actually worthwhile to do so for all of them? ¯_(ツ)_/¯ Gotta pick and choose I guess. Sometimes it’s just not worth it or not terribly likely such a mistake would ever be found. Maybe something like the new Records would be useful here as quick ways to type up these classes with little effort. Though sometimes the API just becomes a pain, especially for things like a “Name” which pretty straightforward and maybe it’s suuuuuper painful to always do something like myCheckingAccount.CustomerName.FullName or whatever. (Though you could add implicit operators or override ToString(). But again, could be a pain making a heavy class just for a freaking name.)
And now I’m rambling in circles.
When I was learning go, one thing I was unsure about was how much you should use the new types underlined by primitives to do the same thing:
Read: Obsession with putting everything in one class and why you should split properties by responsibility into separate classes.
Doesn’t really have anything to do with Primitives imo;
I feel like the article doesn’t quite explain primitive obsession.
Let’s put it a much simpler way: your compiler shouldn’t let you cast the value of a Street to a City, or even more dangerously an AccountNumber to a SocialSecurityNumber, and yet that is exactly how we tend to design classes.
F# has a nicer approach to this, and C# 9 records sort of get us closer. There’s also https://github.com/mcintyre321/ValueOf, which also does things like validation.
Now that’s getting somewhere. Email is no longer a string but instead its own type, checking for whether the address has an @. SocialSecurityNumber can check for its range. And most importantly, you can’t (directly or especially implicitly) cast values that used to be the same type.
I don’t recall why I ended up not using ValueOf, but I think the ideas behind Primitive Obsession are sound.
Really good article! Would be better to delve more into the DDD part and why it’s important for maintainability and how primitive obsession breaks it long term.
C# devs
null reference exceptions