Design Smell: Primitive Obsession by Mark Seemann
This post is the second in a series about Poka-yoke Design - also known as encapsulation.
Many classes have a tendency to consume or expose primitive values like integers and strings. While such primitive types exist on any platform, they tend to lead to procedural code. Furthermore they often break encapsulation by allowing invalid values to be assigned.
This problem has been addressed many times before. Years ago Jimmy Bogard provided an excellent treatment of the issue, as well as guidance on how to resolve it. In relation to AutoFixture I also touched upon the subject some time ago. As such, the current post is mostly a placeholder.
However, it's worth noting that both Jimmy's and my own post address the concern that strings and integers do not sufficiently encapsulate the concepts of Zip codes and phone numbers.
- When a Zip code is represented as a string it's possible to assign values such as null, string.Emtpy, “foo”, very long strings, etc. Jimmy's ZipCode class encapsulates the concept by guaranteeing that an instance can only be successfully created with a correct value.
- When a Danish phone number is represented as an integer it's possible to assign values such as - 98, 0, int.MaxValue, etc. Once again the DanishPhoneNumber class from the above example encapsulates the concept by guaranteeing that an instance can only be successfully created with a correct value.
Encapsulation is broken unless the concept represented by a primitive value can truly take any of the possible values of the primitive type. This is rarely the case.
Design Smell #
A class consumes a primitive type. However, further analysis shows that not all possible values of the type are legal values.
Improved Design #
Encapsulate the primitive value in a Value Object that contains appropriate Guard Clauses etc. to guarantee that only valid instances are possible.
Primitives tend to not be fail-safe, but encapsulated Value Objects are.
Comments
Encapsulation is broken unless the concept represented by a primitive value can truly take any of the possible values of the primitive type. This is rarely the case.
Should a Qty field that only takes positive integers now be represented by a separate class? You seem to be defining encapsulation as compile-safe or something.
Keep in mind that I'm talking about a code smell. This means that whenever you encounter the smell it should make you stop and think. I'm not saying that using a primitive type is always wrong.
With a quantity, and given the constraints of C#, unfortunately there isn't a lot we can do. While we could encapsulate it in a PositiveInteger struct, it wouldn't add much value from a feedback perspective since we can't make the compiler choke on a negative value.
However, a hypothetical PositiveInteger struct would still add the value that it encapsulates the invariant in one place instead of spreading it out all over the code base.
Still, a positive integer is such a basic concept that it never changes. This means that even if you spread Guard Clauses that check for negative values all over your code base, you may not have much of a maintenance burden, since the Guard will never change. Still, you might forget it in a couple of places.
In any case I like the Temperature example above much better because it not only provides safety, but also encapsulates the concept as well as provides conversion logic etc.
I am revisiting an old post in hopes you can shed some light on Value objects and their limits. I am currently trying to implement a Password class, which seems like a perfect example of something that should not be handled as a simple string. I modelled my Password object based on examples you've provided, in that I pass in a string to a constructor and then run an IsValid method to see if the string meets our business rules for passwords (length, types of characters, etc.). That's fine as it is, and I have unit tests to make sure all is well. But there are more business rules to a password. I need to have a privately set DateCreated field, and I need to store the number of days the password is valid, while providing a function to see if the password is still valid based on the DateCreated and the number of days the password is valid. However, adding these things to my value object seems like I'm polluting it. Plus, I want to pass in the number of days valid when the object is created, so now I have two parameters, which causes problems if I want to have an explicit operator. I thought about creating a PasswordPrimitive class and then a Password class that inherits the PasswordPrimitive class, but that seems messy.
If you have any thoughts and/or guidance, I'd appreciate the input.
Regards,
Scott
Scott, thank you for writing. A Value Object can contain more than a single primitive value. The canonical Value Object example is Money, and you often see Money examples where a Money value is composed of an amount and a currency; one example in the literature is Kent Beck's implementation in Test Driven Development: By Example, which contains amount and currency.
Thus, I don't see any intrinsic problem with your password class containing both a string (or a byte array?) and an expiration time.
It's true that you can no longer have a lossless conversion from your Value Object to a primitive value, but that's not a requirement for it to be a Value Object.
(BTW, I hope you don't store passwords, but only the hashes!)