optimize error handling with pattern match in scala

Question

I have a block of code as shown below to handle some exceptions, i use if-else statement but i don't like them nested within each other, wondering if it is possible to use pattern match to make it nicer?

    try {
      if (response.code < 200 || response.code > 299) {
        throw new SearchClientFailure(s"API request failed with code ${response.code}, body ${response.body}")
      } else {
        if (isExceeded(response.body)) {
          throw new SearchClientFailure("Exceed limit")
        } else {
          response.body
        }
      }
    } catch {
      case e: SearchClientFailure =>
        if (queries.isEmpty) {
          throw new SearchClientFailure
        } else {
          logger.warn(s"Failed to update the queries: ${e.message}")
          queries
        }
      case _ =>
        throw new SearchClientFailure
    }

If you throw exceptions then nesting the following if-else isn't really required. You should be able to remove the else without impact. — Dennis Hunziker
– Dennis Hunziker, Commented Jun 14, 2018 at 8:27

C4stor · Accepted Answer · 2018-06-14 08:18:48Z

2

You could do :

   response match {
     case r if (r.code < 200 || r.code > 299) => ...
     case r if (isExceeded(r.body)) => ...
     case r => r.body
   }

Is it nicer ? I'm not 100% sure honestly, I don't really prefer this style to your one.

Btw, depending on what you used you often have access to response.isSuccess() or response.code.isSuccess() instead of testing code values

answered Jun 14, 2018 at 8:18

C4stor

8,0567 gold badges33 silver badges52 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

Tim Over a year ago

This pattern does not work well in this example. If you fill in the ... sections you will find that you end up duplicating code because the first two cases both need to check the queries value and log the result.

C4stor Over a year ago

Hi :-) It's intended to replace the content of the try block, not of the catch one !

cxstam Over a year ago

thanks @C4stor, it is quite nicer, but as mentioned by @Tim, i still can not avoid duplicates in this pattern. but response.isSuccess() is quite good, make the code more readable.

jwvh · Accepted Answer · 2018-06-14 09:39:17Z

1

Rather than take on the overhead of those short throws and catches, I'd be tempted to use Either[String,Response].

Right(response).flatMap{r =>
  if (r.code > 199 && r.code < 300) Right(r)
  else Left(s"API request failed with code ${r.code}, body ${r.body}")
}.flatMap{r =>
  if (isExceeded(r.body)) Left("Exceed limit")
  else Right(r)
}.fold(msg => {
  if (queries.isEmpty) throw new SearchClientFailure
  logger.warn(s"Failed to update the queries: $msg")
  queries
}, _.body)

The only throw required is the one tossed out of this context. Everything else is handled in the code flow.

answered Jun 14, 2018 at 9:39

jwvh

51.3k7 gold badges42 silver badges70 bronze badges

2 Comments

Alexey Romanov Over a year ago

Compared to overhead of multiple flatMaps? I wouldn't be sure which is worse, especially without knowing how often the exception/Left happens...

cxstam Over a year ago

I think the logic behind this code is same as the solution @Tim provided? but with the use of flatMap, the code is much less than the above one. i do like to read some examples like this so as to learn from this code, but from the perspective of readability, i prefer the above solution more personally.

Tim · Accepted Answer · 2018-06-14 10:33:24Z

1

Here is a version that uses Either

val apiResult: Either[String, String] =
  if (response.code < 200 || response.code > 299)
    Left(s"API request failed with code ${response.code}, body ${response.body}")
  else if (isExceeded(response.body))
    Left("Exceed limit")
  else
    Right(response.body)

apiResult match {
  case Right(result) =>
    result
  case Left(message) if queries.nonEmpty =>
    logger.warn(s"Failed to update the queries: $message")
    queries
  case _ =>
    throw new SearchClientFailure
}

The apiResult value stores either the error string or the correct result of the API call. The subsequent match can then retrieve the original error string if required.

This follows the convention that Right is the normal/successful result and Left is the error case or abnormal result.

answered Jun 14, 2018 at 10:33

Tim

27.6k2 gold badges18 silver badges32 bronze badges

1 Comment

cxstam Over a year ago

Thanks @Tim, I do like this solution, seems Either/Left/Right is an error handling version of Option/Some/None. I read through the documentation about it and then I found another pattern match Try/Success/Failure, but it is not perfect for this case, because i have two kinds of failure: api failure but there is already a query exists, or api failure but there is no query exists, with Try/Success/Failure, i may need a nested relation with is what i try to avoid

Collectives™ on Stack Overflow

optimize error handling with pattern match in scala

3 Answers 3

3 Comments

2 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

3 Comments

2 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related