Nick Halase
03/02/2023, 9:28 PMerror(message: String)
messages?
1. System.getenv("GITHUB_SHA") ?: error("GITHUB_SHA is missing")
2. System.getenv("GITHUB_SHA") ?: error("GITHUB_SHA must exist")
dumptruckman
03/02/2023, 9:32 PMNick Halase
03/02/2023, 9:34 PMerror("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?dumptruckman
03/02/2023, 9:34 PMephemient
03/02/2023, 10:53 PM?: error(message)
, I'd use
checkNotNull(System.getEnv("GITHUB_SHA")) { message }
regardless of what the message isJoffrey
03/02/2023, 11:02 PMefemoney
03/03/2023, 1:16 PMKlitos Kyriacou
03/03/2023, 3:53 PMJoffrey
03/03/2023, 4:02 PMcheckNotNull(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.Nick Halase
03/03/2023, 4:05 PMephemient
03/03/2023, 7:32 PMerror()
for IAE, but there is requireNotNull()
. that is one reason I prefer checkNotNull()