Use elvis operator to throw exception Groovy

Question

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?

Answer 1

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.

Answer 2

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.