scala style - how to avoid having lots of nested map

user_47633 0
Viewed 4178

Very often i end up with lots of nested .map and .getOrElse when validating several consecutives conditions

for example:

def save() = CORSAction { request =>
  request.body.asJson.map { json =>
    json.asOpt[Feature].map { feature =>
      MaxEntitiyValidator.checkMaxEntitiesFeature(feature).map { rs =>
        feature.save.map { feature => 
          Ok(toJson(feature.update).toString)
        }.getOrElse {
          BadRequest(toJson(
            Error(status = BAD_REQUEST, message = "Error creating feature entity")
          ))
        }
      }.getOrElse {
        BadRequest(toJson(
          Error(status = BAD_REQUEST, message = "You have already reached the limit of feature.")
        )) 
      }
    }.getOrElse {
      BadRequest(toJson(
        Error(status = BAD_REQUEST, message = "Invalid feature entity")
      )) 
    }
  }.getOrElse {
    BadRequest(toJson(
      Error(status = BAD_REQUEST, message = "Expecting JSON data")
    )) 
  }
}

You get the idea

I just wanted to know if there's some idiomatic way to keep it more clear

scala styles
4 Answers

I agree with Edmondo suggestion of using for comprehension but not with the part about using a validation library (At least not anymore given the new features added to scala standard lib since 2012). From my experience with scala, dev that struggle to come up with nice statement with the standard lib will also end up doing the same of even worst when using libs like cats or scalaz. Maybe not at the same place, but ideally we would solve the issue rather than just moving it.

Here is your code rewritten with for comprehension and either that is part of scala standard lib :

def save() = CORSAction { request =>

  // Helper to generate the error 
  def badRequest(message: String) = Error(status = BAD_REQUEST, message)

  //Actual validation
  val updateEither = for {
    json    <- request.body.asJson.toRight(badRequest("Expecting JSON data"))
    feature <- json.asOpt[Feature].toRight(badRequest("Invalid feature entity"))
    rs <- MaxEntitiyValidator
           .checkMaxEntitiesFeature(feature)
           .toRight(badRequest("You have already reached the limit"))
  } yield toJson(feature.update).toString

  // Turn the either into an OK/BadRequest
  featureEither match {
    case Right(update) => Ok(update)
    case Left(error)   => BadRequest(toJson(error))
  }
}

Explanations

Error handling

I'm not sure how much you know about either but they are pretty similar in behaviour as Validation presented by Edmondo or Try object from the scala library. Main difference between those object regard their capability and behaviour with errors, but beside that they all can be mapped and flat mapped the same way.

You can also see that I use toRight to immediately convert the option into Either instead of doing it at the end. I see that java dev have the reflex to throw exception as far as they physically can, but they mostly do so because the try catch mechanism is unwieldy: in case of success, to get data out of a try block you either need to return them or put them in a variable initialized to null out of the block. But this is not the case is scala: you can map a try or an either, so in general, you get a more legible code if you turn results into error representation as soon as have identified it as they are identified as incorrect.

For comprehension

I also know that dev discovering scala are often quite puzzled by for comprehension. This is quite understandable as in most other language, for is only used for iteration over collections while is scala, it seem to use usable on a lot of unrelated types. In scala for is actually more nicer way to call the function flatMap. The compiler may decide to optimize it with map or foreach but it remain correct assume that you will get a flatMap behavior when you use for. Calling flatMap on a collection will behave like the for each would in other language, so scala for may be used like a standard for when dealing with collection. But you can also use it on any other type of object that provide an implementation for flatMap with the correct signature. If your OK/BadRequest also implement the flatMap, you may be able to use in directly in the for comprehension instead of usong an intermediate Either representation.

For the people are not at ease with using for on anything that do not look like a collection, here is is how the function would look like if explicitly using flatMap instead of for :

def save() = CORSAction { request =>
  def badRequest(message: String) = Error(status = BAD_REQUEST, message)
  
  val updateEither = request.body.asJson.toRight(badRequest("Expecting JSON data"))
    .flatMap { json =>
      json
        .asOpt[Feature]
        .toRight(badRequest("Invalid feature entity"))
    }
    .flatMap { feature =>
       MaxEntitiyValidator
         .checkMaxEntitiesFeature(feature)
         .map(_ => feature)
         .toRight(badRequest("You have already reached the limit"))
     }
     .map { rs =>
       toJson(feature.update).toString
     }

  featureEither match {
    case Right(update) => Ok(update)
    case Left(error)   => BadRequest(toJson(error))
  }
}

Note that in term of parameter scope, for behave live if the function where nested, not chained.

Conclusion

I think that more than not using the right framework or the right language feature, the main issue with the code your provided is how errors are dealt with. In general, you should not write error paths as after thought that you pile up at the end of the method. If you can deal with the error immediately as they occur, that allow you to move to something else. On the contrary, the more you push them back, the more you will have code with inextricable nesting. They are actually a materialization of all the pending error cases that scala expect you to deal with at some point.

user_97754690
Related