camel-snake-kebab icon indicating copy to clipboard operation
camel-snake-kebab copied to clipboard

Should conversion functions return nil on nil input?

Open vincentjames501 opened this issue 5 years ago • 2 comments

If we supply nil to a conversion function, it throws an AssertionError

(csk/->kebab-case-keyword nil)
Execution error (AssertionError) at camel-snake-kebab.core/->kebab-case-keyword (core.cljc:21).
Assert failed: (clojure.core/not (clojure.core/nil? s__20671__auto__))

(cske/transform-keys csk/->kebab-case-keyword {nil "bar"})
Execution error (AssertionError) at camel-snake-kebab.core/->kebab-case-keyword (core.cljc:21).
Assert failed: (clojure.core/not (clojure.core/nil? s__20671__auto__))

We had an issue in production where this happened w/ some unexpected input and since it is an Error and not an Exception some things we're not handled properly.

Does it make sense to return nil on nil input? This feels a bit more idiomatic clojure usage to me.

Error : An Error “indicates serious problems that a reasonable application should not try to catch.”
Exceptions : An Exception “indicates conditions that a reasonable application might want to catch.”

At the very least throwing IllegalArgumentException or something may be better than an AssertionError?

vincentjames501 avatar Apr 12 '20 22:04 vincentjames501

Thanks for opening this issue.

I agree that it is unfortunate that an AssertionError is thrown in this situation.

While I personally dislike how nil is tunneled through many Clojure functions and am bothered by some inconsistencies I agree that returning nil would be idiomatic for the "type-preserving" functions. The "type-converting" functions like ->kebab-case-keyword state that they return a keyword so I am not so sure about returning nil for them.

qerub avatar Apr 26 '20 17:04 qerub

The library has been changed to not throw AssertionError any more. See #70 and #64 for more details. I'm leaving this issue open since I don't think the last word has been said.

qerub avatar Aug 09 '20 15:08 qerub