CODEWITHSUNDEEP
scalascala-option
MNY
MNY

Reputation: 1536

Working with options in Scala (best practices)

I have a method that I wrote to enrich person data by performing an API call and adding the enriched data.

I have this case class:

case class Person(personData: PersonData, dataEnrichment: Option[DataEnrichment])

My method is supposed to return this case class, but I have few filters before, in case person height is not "1.8 m" OR if personId was not found in the bio using the regex, I want to return Person with dataEnrichment = None . My issue is that person height and personId are Options themselves, so it looks like this:

   def enrichPersonObjWithApiCall(person: Person) = {
    
      person.personData.height.map(_.equals("1.8 m")) match {
        case Some(true) =>
          val personId = person.personData.bio flatMap { comment =>
            extractPersonIdIfExists(comment)
          }
          personId match {
            case Some(perId) =>
              apiCall(perId) map { apiRes =>
                Person(
                  person.personData,
                  dataEnrichment = apiRes)
              }
            case _ =>
              Future successful Person(
                person.personData,
                dataEnrichment = None)
          }
        case _ =>
          Future successful Person(
            person.personData,
            dataEnrichment = None)
      }
    }
    
    def extractPersonIdIfExists(personBio: String): Option[String] = {
      val personIdRegex: Regex = """(?<=PersonId:)[^;]+""".r
      personIdRegex.findFirstIn(personBio)
    }
    
    def apiCall(personId: String): Future[Option[DataEnrichment]] = {
      ???
    }
    
    case class DataEnrichment(res: Option[String])
    
    case class PersonData(name: String, height: Option[String], bio: Option[String])

It doesn't seem to be a Scala best practice to perform it like that. Do you have a more elegant way to get to the same result?

Upvotes: 6

Views: 868

Answers (3)

Tim
Tim

Reputation: 27356

Using for is a good way to process a chain of Option values:

def enrichPersonObjWithApiCall(person: Person): Future[Person] =       
  (
    for {
       height <- person.personData.height if height == "1.8 m"
       comment <- person.personData.bio
       perId <- extractPersonIdIfExists(comment)
     } yield {
       apiCall(perId).map(Person(person.personData, _))
     }
  ).getOrElse(Future.successful(Person(person.personData, None)))

This is equivalent to a chain of map, flatMap and filter calls, but much easier to read.

Upvotes: 2

pme
pme

Reputation: 14803

Same idea as Aivean's answer. Just I would use map flatMap and filter.

   def enrichPersonObjWithApiCall(person: Person) = {
     person.personData.height
       .filter(_ == "1.8 m")
       .flatMap{_=>
         val personId = person.personData.bio 
                          .flatMap(extractPersonIdIfExists)
         personId.map(
           apiCall(_)
           .map(apiRes => person.copy(dataEnrichment = apiRes))
         )
      }.getOrElse(Future.successful(person))
    }

It's more readable for me.

Upvotes: 0

Aivean
Aivean

Reputation: 10882

Here, I tried to make it more idiomatic and shorter:

def enrichPersonObjWithApiCall(person: Person) = {
  person.personData.height.collect {
    case h if h == "1.8 m" =>
      val personId = person.personData.bio.flatMap(extractPersonIdIfExists)

      personId.map(
        apiCall(_)
          .map(apiRes => person.copy(dataEnrichment = apiRes))
      )
  }.flatten.getOrElse(
    Future.successful(person.copy(dataEnrichment = None))
  )
}

Basically, the idea is to use appropriate monadic chains of map, flatMap, collect instead of pattern matching when appropriate.

Upvotes: 0

Related Questions