CODEWITHSUNDEEP
groovy
Ordon
Ordon

Reputation: 2895

Use elvis operator to throw exception Groovy

In my code I found situation where my method could return null. In this case I'd rather throw exception than return null. However I don't want to use regular if because in my opinion it look terrible. See code:

class Type{}

@Field Queue<Type> q1 = [] as Queue
@Field Queue<Type> q2 = [] as Queue

Type regularMethod(){
    Type toReturn = q1.poll() ?: q2.poll()
    if(toReturn == null)
        throw new RuntimeException("was null value")
    return toReturn
}
Type myMethod(){
    return q1.poll() ?: q2.poll() ?: exception()
}

Type exception(){
    throw new RuntimeException("was null value")
}

What do you think about using elvis operator here? Is it more readable for you? Or can anyone suggest better solution?

Upvotes: 6

Views: 6802

Answers (4)

Tim James
Tim James

Reputation: 1613

Consider:

q1.poll() ?: q2.poll() ?: {throw new RuntimeException('Both empty')}()

Advantages:

  • No special idiom function where reader has to know what exception() really means.
  • No shared lib function that folks have to know about.
  • Just clear code using language primitives using lazy evaluation.
  • Values before error case, reads sanely from left to right.

Upvotes: 2

Juan Rada
Juan Rada

Reputation: 3766

What about guava preconditions? they are java so they are suitable for groovy too.

Preconditions.checkArgument((q1 && q2, "was null value")

Or using static import

checkNotNull(q1 && q2, "was null value")

Upvotes: 0

Steinar
Steinar

Reputation: 5950

I agree with Jeff, it's a bit hard to read and understand the code. My reasoning is that it hides what's really happening. You can of course make it more clear by improving the method name (to something like throwNewRuntimeException) and perhaps even take the message as a parameter. But I still don't like it. It feels unnecessary to add a new method for this.

I would either have written it exactly as your regularMethod or perhaps turned it around like this:

Type alternativeMethod() {
    if (q1.empty && q2.empty) 
        throw new RuntimeException('Both queues are empty')
    return q1.poll() ?: q2.poll()
}

In this version, I think the meaning is clear and easy to understand. As a bonus you've gotten rid of the clutter that seems to bother you. Even the error message is more descriptive.

Upvotes: 1

Jeff Scott Brown
Jeff Scott Brown

Reputation: 27245

It is of course a matter of preference and style, but I do not like it. The goal isn't to get to the fewest lines of code or the shortest lines of code. The goal should be to end up with concise expressive code. That often happens to be brief, but brevity isn't the primary goal. I think q1.poll() ?: q2.poll() ?: exception() isn't especially easy for the humans to parse.

Upvotes: 3

Related Questions