CODEWITHSUNDEEP
scala
Incerteza
Incerteza

Reputation: 34884

Is there any way to use immutable collections here + make the code look better?

I have to validate some variables manually for some reason and return a map with the sequance of the error messages for each variable. I've decided to use mutable collections for this because I think there is no other choise left:

  val errors = collection.mutable.Map[String, ListBuffer[String]]()
  //field1
  val fieldToValidate1 = getData1()
  if (fieldToValidate1 = "")
    errors("fieldToValidate1") += "it must not be empty!"

  if (validate2(fieldToValidate1))
    errors("fieldToValidate1") += "validation2!"

  if (validate3(fieldToValidate1))
    errors("fieldToValidate1") += "validation3!"

  //field2
  val fieldToValidate2 = getData1()
  //approximately the same steps
  if (fieldToValidate2 = "")
    errors("fieldToValidate2") += "it must not be empty!"

  //.....

To my mind, it look kind of clumsy and there should other elegant solution. I'd also like to not use mutable collections if possible. Your ideas?

Upvotes: 1

Views: 103

Answers (4)

om-nom-nom
om-nom-nom

Reputation: 62835

Although a [relatively] widespread way of doing-the-thing-right would be using scalaz's Validation (as @senia has shown), I think it is a little bit overwhelming approach (if you're bringing scalaz to your project you have to be a seasoned scala developer, otherwise it may bring you more harm than good).

Nice alternative could be using ScalaUtils which has Or and Every specifically made for this purpose, in fact if you're using ScalaTest you already have seen an example of them in use (it uses scalautils underneath). I shamefully copy-pasted example from their doc: 

import org.scalautils._

def parseName(input: String): String Or One[ErrorMessage] = {
  val trimmed = input.trim
  if (!trimmed.isEmpty) Good(trimmed) else Bad(One(s""""${input}" is not a valid name"""))
}

def parseAge(input: String): Int Or One[ErrorMessage] = {
  try {
    val age = input.trim.toInt
    if (age >= 0) Good(age) else Bad(One(s""""${age}" is not a valid age"""))
  }
  catch {
    case _: NumberFormatException => Bad(One(s""""${input}" is not a valid integer"""))
  }
}

import Accumulation._

def parsePerson(inputName: String, inputAge: String): Person Or Every[ErrorMessage] = {
  val name = parseName(inputName)
  val age = parseAge(inputAge)
  withGood(name, age) { Person(_, _) }
}

parsePerson("Bridget Jones", "29")
// Result: Good(Person(Bridget Jones,29))

parsePerson("Bridget Jones", "")
// Result: Bad(One("" is not a valid integer))

parsePerson("Bridget Jones", "-29")
// Result: Bad(One("-29" is not a valid age))

parsePerson("", "")
// Result: Bad(Many("" is not a valid name, "" is not a valid integer))

Having said this, I don't think you can do any better than your current approach if you want to stick with core scala without any external dependencies.

Upvotes: 1

ziggystar
ziggystar

Reputation: 28680

So what is a good type for your check? I was thinking about A => Option[String] if A is the type of your object under test. If your error messages do not depend on the value of the object under test, (A => Boolean, String) might be more convenient.

//for constructing checks from boolean test and an error message
def checkMsg[A](check: A => Boolean, msg: => String): A => Option[String] = 
  x => if(check(x)) Some(msg) else None

val checks = Seq[String => Option[String]](
  checkMsg((_ == ""), "it must not be empty"),
  //example of using the object under test in the error message
  x => Some(x).filterNot(_ startsWith "ab").map(x => x + " does not begin with ab")
)

val objectUnderTest = "acvw"
val errors = checks.flatMap(c => c(objectUnderTest))

Error labels

As I just noted, you were asking for a map with a label for each check. In this case, you need to provide the check label, of course. Then the type of your check would be (String, A => Option[String]).

Upvotes: 1

senia
senia

Reputation: 38045

In case you can use scalaz the best solution for aggregation errors is Validation:

def validate1(value: String) =
  if (value == "") "it must not be empty!".failNel else value.success

def validate2(value: String) =
  if (value.length > 10) "it must not be longer than 10!".failNel else value.success

def validate3(value: String) =
  if (value == "error") "it must not be equal to 'error'!".failNel else value.success

def validateField(name: String, value: String): ValidationNel[(String, String), String] =
  (
    validate1(value) |@|
    validate2(value) |@|
    validate3(value)
  ).tupled >| value leftMap { _.map{ name -> _ } }

val result = (
  validateField("fieldToValidate1", getData1()) |@|
  validateField("fieldToValidate2", getData2())
).tupled

Then you could get optional errors Map like this:

val errors =
  result.swap.toOption.map{
    _.toList.groupBy(_._1).map{ case (k, v) => k -> v.map(_._2) }
  }
// Some(Map(fieldToValidate2 -> List(it must not be equal to 'error'!), fieldToValidate1 -> List(it must not be empty!)))

Upvotes: 0

Windor C
Windor C

Reputation: 1120

Instead of using mutable collections, you can define errors with var and update it in this way.

var errors = Map[String, List[String]]().withDefaultValue(Nil)
errors = errors updated ("fieldToValidate1", errors("fieldToValidate1") ++ List("it must not be empty!"))
errors = errors updated ("fieldToValidate1", errors("fieldToValidate1") ++ List("validation2"))

The code looks more tedious, but it gets out of mutable collections.

Upvotes: 2

Related Questions