0

Let's say I use a Pair in this way:

Pair<Long, Date> signup = getSignup();
System.out.println("User with ID " + signup.getLeft() + " signed up on " + signup.getRight());

Is it a form of Primitive Obsession?

I could have something like

Signup signup = getSignup();
System.out.println("User with ID " + signup.getUsrId() + " signed up on " + signup.getSignupDate());

If it's not a form of primitive obsession, why is that?

  • 2
    I believe your question could be improved if you stated whether you believe it is or not, and your reasoning behind it. A simple "yes" or "no" answer probably won't be too helpful. – Vincent Savard Jan 7 at 15:26
  • A Pair or tuple is sometimes the right abstraction, like gnasher729's answer points out. Some of the advice in the article you linked to is dubious; for example, how on earth is code supposed to "become more flexible thanks to use of objects instead of primitives"? This doesn't make sense and seems like cargo cult nonsense. Code doesn't magically become more "flexible" by using a more specific type instead of a more general abstraction. – Andres F. Jan 8 at 19:05
1

A rule of thumb I draw from the article is that if you are using a Pair/other built in type "just this once" you need to ensure that it can only ever be used "just this once" (within reason).

So let's consider a lambda. It is also something that can start out as being used "just this once".

When you pass lambda as a argument for a filter(), you ensure that (within reason), that lambda will only be used for the filter predicate. You needed to tell the filter() how to filter and your filtering condition was special to that situation, so you used a lambda to provide the predicate. You took advantage of the lambda to provide the predicate as concisely as possible.

If you find that predicate (lambda) being needed in more than one place and you decide to store it in a variable, in my opinion you have probably entered the Primitive Obsession territory. You had a good justification to use that lambda "just this once" before, but the situation has changed. You have nothing to lose by converting it to a method with a descriptive name.

So let's go back to the Pair. It is very unlikely that this Pair will just be used just this once to print out the sign up date. You will likely soon find yourself wanting to save, load, filter etc these signups. Every single time you want to add functionality, you need to take that extra little bit of time to check with yourself what this Pair really means.

A way I would check if I am really allowed to use the Pair or other primitive "just this once" is to ask if can "leak" and someone else can get to use it. Then the question is what can happen when it leaks. When someone gets hold of my filter, all they can do it chuck in a item and get out a true or false. If someone finds this Pair sitting there and starts trying to use it, all sorts of havoc can happen if they don't carefully check what is going on elsewhere with that Pair. For example they may overwrite the information, thinking the pair means something else. When you have a SignUp class with a long userId, someone can easily see when they are working with the userid.

Disclaimer: you should not actually let some other class directly access SignUp.userId directly (have getUserId() instead). I just had it this way for the example.

Another way to check is to interrogate your primitive and ask if it really means what you are trying to intend it to mean. When you have a lambda that takes a list item and spits out a true or false, it is a predicate. You can stick it in anything that wants a predicate.

With your Pair, the two fields only have meaning as a SignUp "mentally". Everywhere you want for your code to treat it as a sign-up, you need to tell it where to get the user id and where to get the data. With a predicate, you just know it's going to decide to give the input a thumbs up or a thumbs down.

  • I see so most of the time this Pair will be used with some logic (a filter). If this data+logic appears in the code then it's a code smell. If I only carry data, no filter or any other logic and it is only used once, it might be better as a Pair but this situation tends to be rare. – ddreian Jan 8 at 7:34
  • Well it is more a concern that you don't want the Pair "leaking" out of where you are using it and ending up somewhere where someone can misunderstand it. I will add a more information in the answer on what I mean by the Pair "leaking" out. – Dev243 Jan 8 at 18:19
  • 1
    Counterpoint: see Rich Hickey's (of Clojure) claim that "All that specificity (of interfaces/classes/types) kills your reuse!" – Andres F. Jan 8 at 19:47
5

While there are cases where a Tuple or Pair is appropriate, I consider it a code smell.

As always with this question its important to note that some people consider 'code smell' to mean something which is always wrong and needs to be refactored.

I think the correct defintion is something that, when you see it, you check to see if it masks or causes some known problem or anti-pattern.

In the case of Tuples its a bit of an edge case as their use is pretty rare. But the anti-pattern of using them extensively as a kind of dynamic object rather than codeing a class seems to come up often enough that it is worth checking for when you see one.

If I saw your example, I would definitely query why a Signup class wasn't used.

  • We are using it A LOT in our code base. That is what motivated me to ask this question. – ddreian Jan 7 at 17:35
1

It is a form of anonymous tuple (x, y). Other such anonymous constructs the now trendy lambda expression as in list.stream().filter(e -> e.name.isEmpty())... (e -> ...), or indeed method parameters f(12, "ab", x).

In these cases a named construct might be better: new Signup(x, y) or Signup.withX(x).withY(y).build() and .filter(Element::hasNoName) and f(signup).

But no, introducing many small classes, especially with getters and setters, perhaps might no smell, but certainly has a negative influence of development productivity - even in IntelliJ IDEA.

If you have repeated constructs with Pair in a source, it might point to applying a wrong data structure; this would not change with the introduction of a data transfer object / tuple class.

Example of a "wrong" Pair: list of (key, value) - here the key should be part of the value data structure or a map used. List of (condition, action) - here a List of boolean predicate might be a better (=higher) abstraction.

  • As intermediate values in transformations from/to maps, lists of pairs (key, value) are not unusual. See: Scala's collection library. Many other languages are the same. – Andres F. Jan 9 at 0:03
1

It depends. If you have an X that comes together with an Y, in other words, if you have a pair, use a pair. If you have an object which consists, just by coincidence, of two items, create a type for that object.

Let’s say you have a car and its price. There’s no point in having a “CarWithPrice” object. Make it a pair. Let’s say you have a teacher, who has a name and teaches a subject. You wouldn’t have a “NameAndSubject” object - however, in this case you would have a Teacher object. Not a pair.

  • 1
    +1 I don't know why you got downvoted, because this answer is right. The OOP obsession with creating needless classes is troubling. – Andres F. Jan 8 at 19:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.