Prefer `ifn?` to `fn?`?

[vemv]

Hey there,

in the following code:

https://github.com/walmartlabs/lacinia/blob/51579c377de226bade32d70f4c1f5b2708d9cbf3/src/com/walmartlabs/lacinia/schema.clj#L360

the spec only accepts fn?s, but not ifn?s. Accepting clojure.lang.IFns would cover the case of creating some 'callable' object through reify, etc.

It's always possible that one is using ifns instead of fns, maybe unknowingly.

Would you expand the spec accordingly?

Thanks - V

[vemv]

Note that vars implement IFn https://github.com/clojure/clojure/blob/653b8465845a78ef7543e0a250078eea2d56b659/src/jvm/clojure/lang/Var.java#L20 , so the code would be simplified (fn? | var? -> ifn?)

[hlship]

That's a good point; the use of ifn? was to prevent people from mistakenly passing in a raw keyword, set, or map, since the function so invoked is passed three arguments.

[vemv]

I see!

One could write a fine-grained spec such as the following:

(spec/and ifn?
          (fn [x]
            (not-any? (fn [c]
                        (instance? (class c) x))
                      [{},
                       #{},
                       [])))

...that should bring the best of both worlds. I intentionally discarded set?, map? etc because a custom reify might satisfy one of those, but only tangentially.

There's also the option of using plain ifn? and let faulty consumers fail (since that'd be quite a corner case).

IMO using spec for ensuring arguments are passed in a specific order is too reminiscing of Java dispatch; a more idiomatic approach would be to accept named args (object or kwargs).

[hlship]

I'm closing this; the current code supports true functions, vars (for REPL-oriented development), and of course, the FieldResolver protocol (the best way to handle reify and friends). The spec as written gives good feedback for the other cases. Unless there is a compelling reason to change from this, I say leave it alone.

[vemv]

the FieldResolver protocol

Sounds fair enough to me then.

Thanks for attending the issue!

[glittershark]

I would vote to reopen this - the code as written doesn't allow multimethods, which is really frustrating in practice (I have to basically eta-expand them)

[hlship]

As a workaround, could you provide a simple function that immediately delegates to the multimethod?

[glittershark]

Yeah, that's what I meant by eta-expand