How do you guys write

error(message: String)

messages? 1.

System.getenv("GITHUB_SHA") ?: error("GITHUB_SHA is missing")

2.

System.getenv("GITHUB_SHA") ?: error("GITHUB_SHA must exist")

I’d probably throw something in to indicate it’s missing from the environment

error("message")

throws an IllegalStateException. I'm curious about the wording of the message. Would you state the variable is missing, or that it should exist?

I mean, I get what you’re asking but also would recommend that you indicate where it is missing from or should exist in. I don’t really have a recommendation for which wording otherwise.

rather than

?: error(message)

, I'd use

checkNotNull(System.getEnv("GITHUB_SHA")) { message }

regardless of what the message is

@ephemient I guess the value of the env variables are useful, so in that case using elvis would probably read much better

I dont think it reads better in this case even if you were to assign it to a variable. One thing to consider is that they throw either IAE or ISE and you have to pick the exception that is correct for your use case

Some general advice on error messages: https://wix-ux.com/when-life-gives-you-lemons-write-better-error-messages-46c5223e1a2f (there are also other sites with similar advice). However, keep in mind that exception messages are not necessarily the same as what would be shown to the end user.

@efemoney for some reason I thought

checkNotNull(x)

was a shortcut

check(x != null)

, but it does in fact return the non-null value, my bad. Both

checkNotNull

and

error

throw ISE, so that is not a distinguishing factor (although I agree in case we need IAE,

require

variants would be nessary). Still, in the case where ISE is needed, I do prefer this:

val sha = System.getenv("GITHUB_SHA") ?: error("GITHUB_SHA is missing")

Over this:

val sha = checkNotNull(System.getenv("GITHUB_SHA")) { "GITHUB_SHA is missing" }

Because we can more clearly see the interesting expression in the happy path, as the error case is handled separately, while using

checkNotNull

kinda mixes both. Similarly, a git diff showing the addition or removal of the check would be more readable (IMO) using the elvis because the whole error case is contiguous in a single piece of code.

@Joffrey thank you!

@Joffrey there's no analogous function to

error()

for IAE, but there is

requireNotNull()

. that is one reason I prefer

checkNotNull()