When is primitive obsession not a code smell?

Question

I have read plenty of articles recently that describe primitive obsession as a code smell.

There are two benefits of avoiding primitive obsession:

1. It makes the domain model more explicit. For example, I can talk to a business analyst about a Post Code instead of a string that contains a post code.

2. All the validation is in one place instead of across the application.

There are plenty of articles out there that describe when it is a code smell. For example, I can see the benefit of removing primitive obsession for a post code like this:

public class Address
{
    public ZipCode ZipCode { get; set; }
}

Here is the constructor of the ZipCode:

public ZipCode(string value)
    {
        // Perform regex matching to verify XXXXX or XXXXX-XXXX format
        _value = value;
    }

You would be breaking the DRY principle putting that validation logic everywhere a zip code is used.

However, what about the following objects:

1. Date of Birth: Check that greater than mindate and less than today's date.

2. Salary: Check that greater than or equal to zero.

Would you create a DateOfBirth object and a Salary object? The benefit is that you can talk about them when describing the domain model. However, is this a case of overengineering as there is not a lot of validation. Is there a rule that describes when and when not to remove primitive obsession or should you always do it if possible?

I guess I could create a type alias instead of a class, which would help with point one above.

Answer 1

Primitive Obsession is using primitive data types to represent domain ideas.

The opposite would be "domain modeling", or perhaps "over engineering".

Would you create a DateOfBirth object and a Salary object?

Introducing a Salary object can be a good idea for the following reason: numbers rarely stand alone in the domain model, they almost always have a dimension and a unit. We don't normally model anything useful if we add a length to a time or a mass, and we seldom get good results when we mix meters and feet.

As for DateOfBirth, probably -- there are two issues to consider. First, creating a non-primitive Date gives you a place to center all of the weird concerns around date math. Many languages provide one out of the box; DateTime, java.util.Date. These are domain agnostic implementations of dates, but they are not primitives.

Second, DateOfBirth isn't really a date time; here in the US, "date of birth" is a cultural construct / legal fiction. We tend to measure date of birth from the local date of a persons birth; Bob, born in California, might have an "earlier" birth date than Alice, born in New York, even though he is the younger of the two.

Is there a rule that describes when and when not to remove primitive obsession or should you always do it if possible.

Certainly not always; at the boundaries, applications are not object oriented. It's fairly common to see primitives used to describe the behaviors in tests.

Answer 2

A possible rule of thumb may depend on the program's layer. For the Domain (DDD) aka Entities Layer (Martin, 2018), this might as well be "to avoid primitives for anything representing a domain/business concept". The justifications are as stated by the OP: a more expressive domain model, business rules validation, making implicit concepts explicit (Evans, 2004).

A type alias can be a lightweight alternative (Ghosh, 2017), and refactored to an entity class when needed. For instance, we may first require that Salary be >=0, and later decide to disallow $100.33333 and anything above $10,000,000 (which would bankrupt the client). The use of Nonnegative primitive to represent Salary and other concepts would complicate this refactoring.

Avoiding primitives may also help avoid over-engineering. Suppose we need to combine Salary and Date of Birth into a data structure: eg, to have fewer method parameters or to pass data between modules. Then we can use a tuple with type (Salary, DateOfBirth). Indeed, a tuple with primitives, (Nonnegative, Nonnegative), is uninformative, whereas some bloated class EmployeeData would hide the required fields among others. The signature in say calcPension(d: (Salary, DateOfBirth)) is more focused than in calcPension(d: EmployeeData), which violates the Interface Segregation Principle. Likewise, a specialised class SalaryAndDateOfBirth seems awkward and is probably an overkill. Later, we may choose to define a data class; tuples and elemental domain types let us defer such decisions.

In an outer layer (eg GUI) it may make sense to "strip" the entities down to their constituent primitives (eg to put into a DAO). This prevents leaking domain abstractions into outer layers, as advocated in Martin (2018).

References
E. Evans, "Domain-Driven Design", 2004
D. Ghosh, "Functional and Reactive Domain Modeling", 2017
R. C. Martin, "Clean architecture", 2018

Answer 3

To be honest: it depends.

There is always the risk of overengineering your code. How widespread will DateOfBirth and Salary be used? Will you only use them in three tightly coupled classes, or will they be used all across the application? Would you "just" encapsule them in their own Type/Class to enforce that one constraint, or can you think of more constraints/functions that actually belong there?

Let's take Salary for example: Do you have any operations with "Salary" (e.g. handling different currencies, or maybe a toString() function)? Consider what Salary is/does when you don't look at it as a simple primitive, and there is a good chance for Salary to be its own class.

Answer 4

Better suffer from Primitive Obsession or being an Architecture Astronaut?

Both cases are pathological, in one case you have too few abstractions, leading to repetition and easily mistaking an apple for an orange, and in the other you forgot to stop with it already and start getting things done, making it hard to get anything done.

As nearly always, you want moderation, a hopefully well-considered middle way.

Remember that a property does have a name, in addition to a type. Also, decomposing an address into its constituent parts might be too constricting if always done in the same way. Not all the world is downtown NY.

Answer 5

If you did have a Salary class it could have methods like ApplyRaise.

On the other hand Your ZipCode class doesn't have to have internal validation to avoid duplicating the validation everywhere you could have a ZipCodeValidator class that could be injected, so if your system is to run both on US and UK adresses you can just inject the correct validator and when you have to handle AUS adresses as well you can just add a new validator.

Another concern is if you have to write data to a database through EntityFramework then it will need to know how to handle Salary or ZipCode.

There isn't a clear cut answer of where to draw the line between how intelligent classes should be, but I will say that I tend to move business logic, like validating, to business logic classes having the data classes being pure data as this seems to work better with EntityFramework.

As for using type aliases, the member/property name should give all information needed about the contents, so I wouldn't use type aliases.

Answer 6

When you do it intentionally, with a purpose. There can be a lot of good purposes, but here are a few:

1. Doing so would overcomplicate the software and make it harder for someone to understand later. This gets into the "Enterprise Java FizzBuzz" example, where there are a dozen classes just to solve the common FizzBuzz interview question. You want the right abstraction structure that makes the problem easier, not an overflowing sequence of types that the next maintainer will hate you for. Having a nice, self-contained solution can be very nice to grok and change. This depends on the purpose and intent.
2. The Type has a structure, but in reality the structure is so complicated or varied that you can't represent all of the business rules or formatting. Things that potentially fall into this camp are addresses and phone numbers (see Falsehoods that Programmers believe about Addresses/Phone numbers). You might think you know what someone will enter, but you don't. But again, this doesn't apply if you are doing it intentionally to restrict to a very specific set of formats. But beware that this is what you are doing. This depends on the purpose and intent.
3. Very similar to the last one, when the primitive is exactly the type you want. "Primitives" in higher level languages really are abstract types. So sometimes that is what you want, and that is fine. Be intentional about it. This depends on the purpose and intent.
4. You intentionally want to limit the use of the value. Like having a private method, you don't want someone to be able to access the type and use elsewhere. Value types are wonderful for re-use elsewhere and having consistency. You might have better a very good internal solution that you don't want to be re-used. Sometimes a tool is very specific for a reason, and that's fine. But make it explicit. This depends on the purpose and intent.
5. When something like performance, or size, or some other optimization matters. There are times when this will affect other constraints you have. Be intentional about why you are doing things. This depends on the purpose and intent.

They all depend on intent. Being a craftsman means that you make choices and tradeoffs. Be aware of alternatives and the reasons why you do things. Some purists will take a hammer and think everything is a nail, but the world doesn't work like that. But there are lots of common problems for which a hammer is a wonderful thing.

Answer 7

(What the question probably really is)

When is the use of primitive type not a code smell?

(Answer)

When the parameter doesn't have rules in it - use a primitive type.

Use primitive type for the likes of:

htmlEntityEncode(string value)

Use object for the likes of:

numberOfDaysSinceUnixEpoch(SimpleDate value)

The latter example have rules in it, i.e., the object SimpleDate is comprised of Year, Month, and Day. Through the use of Object in this case, the concept of SimpleDate being valid can be encapsulated within the object.

Answer 8

Apart from the canonical examples of email addresses or zip codes given elsewhere in this question, Where I find refactoring away from Primitive Obsession can be particularly helpful is with entity IDs (see https://andrewlock.net/using-strongly-typed-entity-ids-to-avoid-primitive-obsession-part-1/ for an example of how to do it in .NET).

I have lost count of the number of times a bug has crept in because a method had signature like this:

int leaveId = 12345;
int submitterId = 23456;
int approverId = 34567;

SubmitLeaveApplication(leaveId, approverId, submitterId);

public void SubmitLeaveApplication(int leaveId, int submitterId, int approverId) {
  // implementation here
}

Compiles just fine, and if you're not rigorous with your unit testing, it may pass that too. However, refactor those entity IDs into domain-specific classes, and hey presto, compile-time errors:

LeaveId leaveId = 12345;
SubmitterId submitterId = 23456;
ApproverId approverId = 34567;

SubmitLeaveApplication(leaveId, approverId, submitterId);

public void SubmitLeaveApplication(LeaveId leaveId, SubmitterId submitterId, ApproverId approverId) {
  // implementation here
}

Imagine that method scaled up to 10 or more parameters, all of int data type (never mind the Long Parameter List code smell).It gets even worse when you use something like AutoMapper to swap between domain objects and DTOs, and a refactoring that you do doesn't get picked up by the automagic mapping.

Answer 9

You would be breaking the DRY principle putting that validation logic everywhere a zip code is used.

On the other hand, when dealing with many different countries and their different zipcode systems, that means you cannot validate a zipcode unless you know the country in question. So your ZipCode class needs to also store the country.

But do you then separately store the country as both part of the Address (which the zipcode is also part of), and part of the zipcode (for validation)?

• If you do, you're violating DRY as well. Even if you don't call it a DRY violation (because each instance serves a different purpose), it's still needlessly taking up extra memory, on top of opening the door to bugs when the two country values are different (which they logically never should be).
  • Or, alternatively, it leads to you needing to synchronize the two data points to ensure that they are always the same, which suggests that you should really store this data in a single point anyway, thus defeating the purpose.
• If you don't, then it's not a ZipCode class but an Address class, which again will contain a string ZipCode which means we've come full circle.

For example, I can talk to a business analyst about a Post Code instead of a string that contains a post code.

The benefit is that you can talk about them when describing the domain model.

I don't understand your underlying assertion that when a piece of information has a given variable type, that you're somehow obligated to mention that type whenever you're talking to a business analyst.

Why? Why are you unable to simply talk about "the zipcode" and completely omit the specific type? What kind of discussions are you having with your business (not technical!) analyst where the type of the property is quintessential to the conversation?

Where I'm from, postcodes are always numeric. So we have a choice, we could store it as an int or as a string. We tend to use a string because there's no expectation of mathematical operations on the data, but never has a business analyst told me that it needed to be a string. That decision is left up to the developer (or arguably the technical analyst, though in my experience they don't directly deal with the nitty gritty).

A business analyst does not care about the data type, as long as the application does what it's expected to do.

---

Validation is a tricky beast to tackle, because it relies on what humans expect.

For one, I don't agree with the validation argument as a way to show why primitive obsession should be avoided, because I don't agree that (as a universal truth) data always needs to be validated at all times.

For example, what if this is a more complicated lookup? Rather than a simple format check, what if your validation entails contacting an external API and waiting for a response? Do you really want to force your application to call this external API for every ZipCode object you instantiate?
Maybe it's a strict business requirement, and then it's of course justifiable. But this is not a universal truth. There will be plenty of use cases where this is more a burden than it is a solution.

As a second example, when entering your address in a form, it's common to enter your postcode before your country. While it's nice to have immediate validation feedback in the UI, it would actually be a hindrance to me (as a user) if the application alerted me to a "wrong" zipcode format, because the real source of the issue is (e.g.) that my country isn't the country that is selected by default, and thus the validation happened for the wrong country.
It's the wrong error message, which distracts the user and causes needless confusion.

Just like how perpetual validation isn't a universal truth, neither are my examples. It's contextual. Some application domains require data validation above all else. Other domains do no put validation that high on the list of priorities because the hassle it brings with it conflicts with their actual priorities (e.g. user experience, or the ability to initially store faulty data so it can be corrected instead of never allowing it to be stored)

Date of Birth: Check that greater than mindate and less than today's date.
Salary: Check that greater than or equal to zero.

The problem with these validations is that they are incomplete, redundant or indicative of a much larger problem.

Checking that a date is greater than the mindate is redundant. The mindate literally means that it is the smallest possible date. Besides, where do you draw the line of relevance? What's the point in preventing DateTime.MinDate but allowing DateTime.MinDate.AddSeconds(1)? You're cherrypicking a particular value that is not particularly wrong compared to many other values.

My birthday is Jan 2nd 1978 (it isn't, but let's assume it is). But let's say the data in your application is wrong, and instead it says my birthday is:

• Jan 1st 1978
• Jan 1st 1722
• Jan 1st 2355

All of these dates are wrong. None of them is "more right" than the other. But your validation rule would only catch one of these three examples.

You've also completely omitted the context of how you're using this data. If this is used in e.g. a birthday reminder bot, I'd say the validation is pointless since there is no particular bad consequence to filling in the wrong date.
On the other hand, if this is government data and you need the birthdate to authenticate someone's identity (and failure to do so leads to bad consequences, e.g. denying someone social security), then the correctness of the data is paramount and you need to fully validate the data. The proposed validation you have now is not adequate.

For a salary, there is some common sense in that it cannot be negative. But if you realistically expect that nonsensical data is being entered, I would suggest that you investigate the source of this nonsensical data. Because if they can't be trusted to enter sensical data, you also can't trust them to enter correct data.

If instead the salary is calculated by your application, and somehow it's possible to end up with a negative (and correct) number), then a better approach would be to do Math.Max(myValue, 0) to turn negative numbers into 0, rather than fail the validation. Because if your logic decided that the outcome is a negative number, failing the validation means it will have to redo the calculation, and there's no reason to think that it will come up with a different number the second time.
And if it does come up with a different number, that again leads you to suspect that the calculation is not consistent and therefore cannot be trusted.

This is not to say that validation isn't useful. But pointless validation is bad, both because it takes effort while not really solving a problem, and giving people a false sense of security.