A modern programming language that makes developers happier.

kotlinlang

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

<@UJQB4DJF7> 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.

<@U5EFLTPJ5> 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.

In any case, this discussion is orthogonal to the original question, sorry <@UL50LG7M0>. Both messages look OK to me, but I would suggest giving more information in the message about why the env variable was expected to be there if it's possible. This would help debug this invalid state.

<@UBXMWAA69> there's no analogous function to `error()` for IAE, but there is `requireNotNull()`. that is one reason I prefer `checkNotNull()`