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

Question

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?

Are you looking for vanilla scala? – om-nom-nom Feb 11 '14 at 08:31 — om-nom-nom, Feb 11 '14 at 08:31

score 2 · Accepted Answer · answered Feb 11 '14 at 08:33

2

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.

answered Feb 11 '14 at 08:33

Windor C

1,120
8
17

are these mutable map and list? – Incerteza Feb 11 '14 at 08:39
won't it cause the exception if the key fieldToValidate1 doesn't exist? – Incerteza Feb 11 '14 at 08:42
@Alex the map and list are not mutable. `withDefaultValue` method prevents the exception. – Windor C Feb 11 '14 at 08:48
I wonder, why does this work: `errors = errors updated ("fieldToValidate1", "it must not be empty!" :: errors("fieldToValidate1"))` -- cons operator? – Incerteza Feb 11 '14 at 09:04
@Alex `updated` is a method of `scala.collection.immutable.Map`. The signature is `updated[B1 >: B](key: A, value: B1): Map[A, B1]`. – Windor C Feb 11 '14 at 09:13
I mean, isn't `errors("fieldToValidate1") ++ List("it must not be empty!")` the same as `"it must not be empty!" :: errors("fieldToValidate1")`? If not, what's the difference? I don't see any difference, but it's not working. – Incerteza Feb 11 '14 at 09:32
@Alex Both of them works fine to me. as for the difference, the latter is more efficient if the order of elements doesn't matter so much. – Windor C Feb 11 '14 at 09:41
2

don't see much point of immutable collection with var over mutable collection with var – om-nom-nom Feb 11 '14 at 10:26
At least make a function and use `errors += f -> (errors(f) :+ e)`. – som-snytt Feb 11 '14 at 10:38
@om-nom-nom you can see http://stackoverflow.com/questions/11386559/val-mutable-versus-var-immutable-in-scala – Windor C Feb 11 '14 at 10:48
@WindorC so without writing a caution *do this is only if you keep variable in function scope* you only worstened a situation – om-nom-nom Feb 11 '14 at 11:21
@WindorC in the above comment you linked question with *advantages of var with mutable collection* over val/mutable. The top answer instructs you to increase referential transparency (thus, stick to val/mutable collection as a general principle), **but** it is better to use var/immutable in case when you're constructing collection in some function and passing it in outside world. From op question it is not clear whether op has the later situation, so there is a possibility that var/immutable will decrease ref. transparency, so would be better to make a notion where var/immutable will be win – om-nom-nom Feb 12 '14 at 13:51
@om-nom-nom I think what the questioner wants is returning the constructed errors. There is no other reference to `errors` during the constructing. And returning immutable collections could be a better choice, at least I think so. – Windor C Feb 13 '14 at 02:06

score 1 · Answer 2 · answered Feb 11 '14 at 11:45

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]).

om-nom-nom · Answer 3 · 2014-02-11T16:09:09.920

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.

score 0 · Answer 4 · answered Feb 11 '14 at 09:02

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!)))

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

4 Answers4

Error labels