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?

ddreian
  • 111
  • 5
  • 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 07 '19 at 15:26
  • [Class AbstractMap.SimpleEntry](https://docs.oracle.com/javase/7/docs/api/java/util/AbstractMap.SimpleEntry.html) – Robert Harvey Jan 07 '19 at 22:02
  • A `Pair` or tuple is sometimes the right abstraction, like [gnasher729's answer points out](https://softwareengineering.stackexchange.com/a/385097/16247). 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 08 '19 at 19:05

5 Answers5

5

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

As always with this question, it's 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 definition 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, it's 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 coding 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.

Pang
  • 313
  • 4
  • 7
Ewan
  • 70,664
  • 5
  • 76
  • 161
  • We are using it A LOT in our code base. That is what motivated me to ask this question. – ddreian Jan 07 '19 at 17:35
  • Pair, Tuple, DTO, or Java 14 Records are all just fancy names for data structures. Real objects encapsulate. These don't. Use data structures when throwing data over a wall that wont let you move the methods that go with it. Use objects when you can move the methods. – candied_orange Feb 14 '21 at 19:35
  • Yeah that sums it up well i think – Ewan Feb 14 '21 at 21:09
3

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.

Dev243
  • 86
  • 3
  • 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 08 '19 at 07: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 08 '19 at 18:19
  • 1
    Counterpoint: see Rich Hickey's (of Clojure) claim that ["All that specificity (of interfaces/classes/types) kills your reuse!"](https://softwareengineering.stackexchange.com/q/199217/16247) – Andres F. Jan 08 '19 at 19:47
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.

Joop Eggen
  • 2,011
  • 12
  • 10
  • 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 09 '19 at 00: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.

Now you are using C++ where a pair is just a built-in type. In Swift, tuples (and triples etc.) is one of the five fundamental type categories (class, struct, enum, tuple and closure) that are part of the language type system. So anyone calling it a “code smell” will be laughed off, and tuples are much easier to use.

gnasher729
  • 42,090
  • 4
  • 59
  • 119
  • 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 08 '19 at 19:02
0

To me, I would use a Signup class, not a Pair 2-tuple. Because, even if the implementation of Signup is actually a Pair right now, someday it might not be.

A well-designed class is a human-obvious description of what something "is," not merely "how it is (presently ...) constructed." A method like getUserId() plainly indicates what the method does, whereas getLeft() conveys nothing to the reader. It also can return a rigorously-defined type, where a generic method like getRight() has no such opportunity.

It also gives you opportunities to use explicit types to detect errors at compile-time. When the object code is generated, it will all be about the same, but here you're letting the compiler do a better job for you and to keep you clear of more bugs. Java has a very strong type system: plan to use it to maximum advantage.

Mike Robinson
  • 1,765
  • 4
  • 10