A linter idea: When a defmulti is defined as a keyword, warns in case the arity of one of the methods is greater than 1. Otherwise keyword-as-a-function treats the second argument as a default value. This mistake in my code caused an out of memory error due to the caching mechasim of multi-methods. https://ask.clojure.org/index.php/10532/memory-leak-using-the-default-method-of-a-multimethod
Thanks. It's a pretty niche problem I think, but I'll keep it in mind
speaking of: thoughts on having a linter that checks that a defmulti
dispatch fn is a var? if you provide a fn, trying to later change that fn in a repl is tricky
also sounds pretty niche :)
do you mean: give a warning so you can remove it before going into production?
i guess you could consider it niche, though i suspect it's good practice everywhere defmulti
s are used
it'd definitely be a warning, though in production theoretically fn dispatch values are ok since you shouldn't be mucking around in a repl in production 😛
I don't understand which of the two you find good practice
;; this is better than ...
(defn dispatch
[args]
<some logic>)
(defmulti my-multi #'dispatch)
;; ... this
(defmulti my-multi (fn [args] <some logic>))
and you want clj-kondo to warn on the latter? I don't think I agree with this, but maybe I just haven't run into trouble enough with re-defining multi-methods. In my experience this is pretty rare.
i think it would be useful, we've been hit a couple of times by it. the solution is to just reload your repl, but still it's annoying
@didibus The reason you don’t want to redefine multimethods is because this would remove all extensions in other namespaces potentially. For this same reason (def my-multi nil)
is not (always) a good idea. I’m using the trick of @ddouglass as well. I don’t use defmulti in performance heavy areas normally so (defmulti my-multi #'dispatch)
is a better approach for my workflow
I saw that, normally actually I have it in a comment block above and I just know to evaluate it in my REPL after I changed my dispatch FN
Most of the time, when I change my dispatch-fn, it's because it didn't work and I don't mind re-evaluating the defmethods afterwards
But I see the advantage of Var indirection over it, appart for performance, seems you get all the benefits and none of the negatives
yeah for me it was the best approach. I have a project where I have 5 or more namespaces extending one multimethod. So this means i have to reload those 5 namespaces. I have had long debugging sessions because of this. So I decided to always go for the var approach
Maybe the idea of having a macro that would remove the indirection in production could be useful here. To remove the performance argument
Originally I thought you were arguing the opposite: don't use a var because this is worse for performance :)
I guess you could even write (when dev? #'foo foo)
.
Ah yah, rereading what I typed I can see the confusion, sorry bout that :P
You don't need to reload the REPL
Just do: (def my-multi nil)
and then re-evaluate the defmulti and it's going to work
oh interesting, i hadn't thought of that
i'll have to rethink my stance then (though i suspect i'll still lean towards the "use a var" side)
To be honest, I don't know why defmulti behaves as it does, but for some reason they went above and beyond to prevent it from being redefined and I wonder why, like is there a good reason
What I tend to do is this:
(def my-multi nil)
(defmulti my-multi (fn [...] ...))
That way when I reload the namespace in the REPL it reloads the defmulti as well in case I changed the dispatch-fn. But in prod I'm not messing with the standard defmulti in case there's a reason it is how it is.very niche but when it happened it cost me 5 days of painful of investigation with all kind of JVM profiling tools