2
\$\begingroup\$

The exercise description:

Given a ‘words.txt’ file (attached) containing a newline-delimited list of dictionary words, please implement a small command line application capable to return all the anagrams from ‘words.txt’ for a given word. You can use any Scala library of your choice, no frameworks though.

Note: the file contains words that are sometimes Uppercased or have random upper/lowercasing. Checking for an anagram should not consider casing.

My solution:

Main.scala

import scala.io.Source
import scala.util.Using

object Main extends App {

  def isAnagram(word: String, anagram: String) : Boolean = word.toLowerCase.toSeq.sorted.unwrap == anagram.toLowerCase.toSeq.sorted.unwrap

  print("Type the word for which you wish to search anagrams: ")

  val word = scala.io.StdIn.readLine()

  Using(Source.fromResource("words.txt")){ source =>
    print("Anagrams found: ")
    val anagrams = source.getLines().filter(possibleAnagram => isAnagram(word, possibleAnagram)).toList
    println(anagrams match {
      case _ :: _ => anagrams.mkString(", ")
      case _ => "none"
    })
  }
}

MainTest.scala

import org.scalatest.FunSuite
import org.scalatest.prop.TableDrivenPropertyChecks

class MainTest extends FunSuite with TableDrivenPropertyChecks {
  test("anagram") {
    val parametersTable =
      Table(
        ("listen", "silent"),
        ("stressed", "desserts"),
        ("Dusty", "Study")
      )

    forAll (parametersTable) { (word: String, anagram: String) =>
      assert(Main.isAnagram(word, anagram) === true)
    }
  }

  test("not an anagram") {
    val parametersTable =
      Table(
        ("listen", "desserts"),
        ("stressed", "silent"),
        ("testing", "study")
      )

    forAll (parametersTable) { (word: String, anagram: String) =>
      assert(Main.isAnagram(word, anagram) === false)
    }
  }
}

Using scalatest library for writing the tests.

Stuff I considered when writing the code:

  • having data driven tests that cover all the basic corner cases
  • performance - the data is read from the file and processed in a streaming fashion
  • readability and conciseness - I tried to apply this as much as my Scala knowledge allowed me

Feedback I received from a third party:

Was told that this code is not good enough without getting any concrete explanation.

\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! Your question would benefit from an explanation of how you went about solving the problem (i.e. how your code works and perhaps why you did it that way). \$\endgroup\$ – VisualMelon Aug 19 '19 at 21:57
0
\$\begingroup\$

I'm going to review your Main code and let someone else address the Test code.

I'll start with a few minor issues.

Breaking up long lines often makes them easier to scan/parse/comprehend.

def isAnagram(word: String, anagram: String) : Boolean =
  word.toLowerCase.toSeq.sorted.unwrap == anagram.toLowerCase.toSeq.sorted.unwrap

Also, you're using .toSeq to get to the non-deprecated .sorted method, but there's no reason to .unwrap it to get back to a String. The == comparison will work on a WrappedString.

Using.apply() returns a Try[A], which your code ignores. If an exception is thrown (words.txt isn't where it should be?) then your app silently returns without reporting any errors. You might want to consider Using.resource() instead. It won't try to catch any exceptions.

Using pattern matching just to determine if anagrams is populated or not seems a bit bizarre. In fact, it doesn't need to be a List at all.

val anagrams = source.getLines()
                     .filter(isAnagram(word, _))
                     .mkString(", ")

if (anagrams.isEmpty) println("none")
else                  println(anagrams)

A big, big, problem with your algorithm is that the user-input, word, is processed for comparison (toLowerCase and toSeq and sorted) for every word in the dictionary. (*Ouch!*) It should only be done once.

But my biggest issue with the design is your choice of user experience (UX). Prompting for user input has many pitfalls. I won't go into the whole rigamarole about using the IO monad in an FP context, instead I'll just point out that simplifying the user I/O can greatly enhance the app's utility.

What if the user wants to count the number of anagrams for a given word? If your app had a small and simple usage profile (usage: anagrams <word>) then the task would be easy.

%%> anagrams Star | wc -w
5
%%>

From there you can imagine simple feature enhancements to make it more useful.

%%> anagrams Star bets ENDING
Star: arts, rats, tars, tsar
bets: best
ENDING: ginned
%%>
|improve this answer|||||
\$\endgroup\$

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.