4
\$\begingroup\$

I have a list of words (with repetitions), and I intend to find out the longest words amongst them, along with their length. My question is to know if my implementation is too verbose, is using too many operators/functions and if so, what are the better, more idiomatic, readable alternatives.

Given the list below, the expected output is

 Max length=3, words are "mnp","pqr","jkl","abc","xyz"
    val line = List("a","ab","abc","xyz","mnp","ac","d","b","ab","bc","bd","be","bf","b","abc","abc","pqr","mnp","jkl","a","b")

    val lengthAndTheWords = line.map(x => (x.length,x)).groupBy(x => x._1).mapValues(x => x.toSet)
    val longest = lengthAndTheWords.toSeq.sortBy(key => -key._1).head

    println("Max length =" + 
            longest._1 +
            ", words are" + 
            longest._2.map(nextEntry => nextEntry._2).mkString(","))

(With an Ideone running the code)

I consider myself an intermediate level Scala programmer; so, I am looking for all tips to improve my skills in Scala.

\$\endgroup\$
7
  • 1
    \$\begingroup\$ Does your example word list consist of string literals? \$\endgroup\$ Commented Jun 11, 2014 at 11:30
  • \$\begingroup\$ I edited an "s" out: "along with their lengths" -> "along with their length". I am guessing you want the longest word(s) with its(their) length. \$\endgroup\$
    – toto2
    Commented Jun 11, 2014 at 20:12
  • \$\begingroup\$ What's xyz? What's pqr? etc. As it currently stands, this code is a bit invalid. \$\endgroup\$ Commented Jun 11, 2014 at 20:47
  • \$\begingroup\$ This code is incomplete, and does not produce a result. It creates a map, but does not select the longest 'anything'. \$\endgroup\$
    – rolfl
    Commented Jun 11, 2014 at 21:26
  • \$\begingroup\$ @rolfl - I have edited the post following your comments. I have mentioned explicitly what is the expected output for the given input set. Thanks for your comments. Can this question now be taken off the hold please? \$\endgroup\$
    – Nirmalya
    Commented Jun 13, 2014 at 6:16

1 Answer 1

2
\$\begingroup\$

Whenever I only use the extracted value once in a simple group / map / sortBy monadic operations I use the _ placeholders. I think it makes the code a little more compact and readable, especially if it was just a placeholder value like x

.groupBy(x => x._1) becomes .groupBy(_._1)

.mapValues(x => x.toSet) becomes .mapValues(_.toSet) or even mapValues toSet if you like infix

It's not a universal rule by any means, sometimes you really need a meaningful name so that another coder could make sense of it.

Edit Also you can go straight to the group by and remove redundant counts

val oneLiner = line.groupBy(_.length).mapValues(_.toSet).maxBy(_._1)

//> oneLiner  : (Int, scala.collection.immutable.Set[String]) = (3,Set(abc, pqr, mnp, jkl, xyz))
\$\endgroup\$
2
  • \$\begingroup\$ thanks for the one-liner. I always felt that mine was verbose. As for the placeholder '_', I prefer to put a parameter explicitly because it helps me to 'see' the function logic somewhat clearly, but that is primarily due to my (slowly receding) unease about the functional idioms. Many thanks. \$\endgroup\$
    – Nirmalya
    Commented Jun 18, 2014 at 4:56
  • \$\begingroup\$ I think Odersky was real smart to leave both ways in there so that there's an option that's not off-putting to beginners. The more I went down the rabbit hole I preferred the placeholders. \$\endgroup\$
    – Gangstead
    Commented Jun 18, 2014 at 14:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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