Should x._1,x._2... syntax be avoided?

Asked 9/12, 2012 at 19:18 Answered 1/6, 2020 at 13:20

I'm just starting out in Scala. I find myself using tuple variables a lot.

For example, here's some code I wrote:

/* Count each letter of a string and return in a list sorted by character
 * countLetter("test") = List(('e',1),('s',1),('t',2))
*/
def countLetters(s: String): List[(Char, Int)] = {
  val charsListMap = s.toList.groupBy((c:Char) => c)
  charsListMap.map(x => (x._1, x._2.length)).toList.sortBy(_._1)
}

Is this tuple syntax (x._1, x._2 etc) frowned upon by Scala developers?

Boling answered 9/12, 2012 at 19:18 Comment(1)

I'll think it's okay and desired by the Scala developers. :) – Platter 9/12, 2012 at 19:25

Are the tuple accessors frowned upon by Scala developers?

Short answer: no.

Slightly longer (by one character) answer: yes.

Too many _n's can be a code smell, and in your case the following is much clearer, in my opinion:

def countLetters(s: String): List[(Char, Int)] =
  s.groupBy(identity).mapValues(_.length).toList.sortBy(_._1)

There are lots of methods like mapValues that are specifically designed to cut down on the need for the noisy tuple accessors, so if you find yourself writing _1, etc., a lot, that probably means you're missing some nice library methods. But occasionally they're the cleanest way to write something (e.g., the final _1 in my rewrite).

One other thing to note is that excessive use of tuple accessors should be treated as a nudge toward promoting your tuples to case classes. Consider the following:

val name = ("Travis", "Brown")

println("Hello, " + name._1)

As opposed to:

case class Name(first: String, last: String)

val name = Name("Travis", "Brown")

println("Hello, " + name.first)

The extra case class definition in the second version buys a lot of readability for a single line of code.

Killifish answered 9/12, 2012 at 19:30 Comment(7)

This is good advice, but I consider mapValues and friends a code smell also--more of a library smell than a user-land smell, but still, lots of overly specific methods that need to be remembered and maintained. I'd rather have remappers on tuple elements: _.map(_.f2(_.length)) – Ultramarine 9/12, 2012 at 19:45

@RexKerr: I don't know—having a separate method for mapValues clearly captures the idea that _.map(_.f2(_.length)) is a very different animal from _.map(_.f1(_.length)) on maps. I'm with the language designers on this one. – Killifish 9/12, 2012 at 19:59

Pleas mention in your answer that mapValues is lazy. It reduces memory usage, but evaluates at every access to value. – Terrapin 9/12, 2012 at 20:2

@senia: But you'll always need to force it before sorting (e.g., with the toList here). I don't think I've ever been bitten by forgetting the laziness of mapValues. – Killifish 9/12, 2012 at 20:9

@TravisBrown Uh, I have. It was very nasty; it happened in some GUI code and caused a memory leak. Took me at least a day to find the cause of the leak. – Shoddy 9/12, 2012 at 20:45

I like the idea of using case classes instead of tuples (often), but mapValues is really a special case - it's the perfect answer for a Map, but that's it (I'm curious about the "tons of methods like mapValues). – Miasma 9/12, 2012 at 22:7

I'd like to point out that, while I agree with the answer, I think that the usage x._1, x._2 and similar is very clear for pure, i.e. mathematical usage of tuples. Actually, I event tend to replicate that in Java (since I know Scala). – Rosefish 10/12, 2012 at 0:30

There is a better solution then x._N. Common way to work with tuples is pattern matching:

charsListMap.map{case (a, b) => (a, b.length)}

You also may take a look at scalaz, there are some instruments for tuples:

import scalaz._
import Scalaz._

scala> (1, "a") bimap (_ + 2, _ + 2)
res0: (Int, java.lang.String) = (3,a2)

scala> ('s, "abc") :-> { _.length }
res1: (Symbol, Int) = ('s,3)

Terrapin answered 9/12, 2012 at 19:33 Comment(5)

@Jean-PhilippePellet: Not the downvoter, but this doesn't answer the real question, and mapValues is more idiomatic, anyway. – Killifish 9/12, 2012 at 19:41

@TravisBrown: There is mapValues in your answer and it's the way to do for Map. But pattern matching is more generic solution. It work in almost all cases for tuples. – Terrapin 9/12, 2012 at 19:47

@TravisBrown It is worth noting that mapValues creates a wrapper around the original map, thus introducing overhead each time you get a value, whereas senia's solution creates a new direct map. – Skylar 9/12, 2012 at 20:21

@senia: That's a fair point, and if your answer had said that, I would have upvoted it. – Killifish 9/12, 2012 at 20:29

@Jean-PhilippePellet: I'm not sure that's relevant here (because of the toList), especially since the asker is a self-professed beginner and in most cases mapValues does exactly the right thing. But you have my blessing if you want to edit my answer, of course. – Killifish 9/12, 2012 at 20:34

Starting in Scala 3, with the parameter untupling feature, the following will become an alternative for .map(x => x._1 -> x._2.length):

.map(_ -> _.length)

and thus, your example becomes:

"test".toList.groupBy(identity).map(_ -> _.length).toList.sortBy(identity)
// List(("e", 1), ("s", 1), ("t", 2))

Concerning your example more specifically and starting in Scala 2.13, you could also use groupMapReduce which (as its name suggests) is an equivalent of a groupBy followed by mapValues and a reduce step:

"test".groupMapReduce(identity)(_ => 1)(_ + _).toList.sortBy(identity)

Trichloromethane answered 1/6, 2020 at 13:20 Comment(0)

Recommended topics

Hot tags