technomancy / robert-hooke

Hooke your Clojure functions!
Other
359 stars 25 forks source link

Duplicate hooks added to remote ns function #3

Closed pauldorman closed 12 years ago

pauldorman commented 12 years ago

Hi, I'm using robert-hooke to add some extra functionality to the Korma library (mostly as an exercise). I'm using Emacs with clojure-jack-in, Leiningen, and Clojure 1.3.0, Korma 0.3.0-alpha12, and robert/hooke 1.1.2, all installed from Clojars.

Incidentally, the 1.1.2 version of robert-hooke in Clojars seems a bit inconsistent with the Github code, with the clear-hooks function not included. That's not a major issue however and I've replaced the code in the jar file as a workaround.

When I include the examine and doubler functions from the examples into my project's namespace, I can compile my code repeatedly and the function works as expected in the repl. When I add a hook to #'korma.core/exec however, the hook gets added every time I compile my code, so I get repeated executions.

Here's an example (with (add-hook #'korma.core/exec hooke-test)), where compile means I've compiled my code:

my.namespace> (meta @#'korma.core/exec)
nil

**compile**
my.namespace> (meta @#'korma.core/exec)
{:robert.hooke/original #<core$exec korma.core$exec@547c9586>, :robert.hooke/hook #<Atom@f598d5f: (#<core$hooke-test my-ns.core$hooke-test@d818ff0>)>}

**compile**
my.namespace> (meta @#'korma.core/exec)
{:robert.hooke/original #<core$exec korma.core$exec@547c9586>, :robert.hooke/hook #<Atom@f598d5f: (#<core$hooke-test my-ns.core$hooke-test@28da431b> #<core$hooke-test my-ns.core$hooke-test@d818ff0>)>}

**compile**
my.namespace> (meta @#'korma.core/exec)
{:robert.hooke/original #<core$exec korma.core$exec@547c9586>, :robert.hooke/hook #<Atom@f598d5f: (#<core$hooke-test my-ns.core$hooke-test@7305830a> #<core$hooke-test my-ns.core$hooke-test@28da431b> #<core$hooke-test my-ns.core$hooke-test@d818ff0>)>}

A potential workaround would be to add a (clear-hooks #'korma.core/exec) call before I add the hook(s), but unfortunately the clear-hooks function doesn't like it when there are none to clear. I can log a separate issue for this if you like.

I've experimented with various solutions, but I suspect a deeper implementation issue prevents a trivial workaround. Correct behavior can be obtained by modifying the test in add-unless-present to "(if-not (some #(= (str (class f)) (str (class %))) coll)". I'm certain there's a better solution.

Thanks for your time and help.

technomancy commented 12 years ago

A potential workaround would be to add a (clear-hooks

'korma.core/exec) call before I add the hook(s), but unfortunately

the clear-hooks function doesn't like it when there are none to clear. I can log a separate issue for this if you like.

Definitely want to fix this; having an issue here would be good.

I've experimented with various solutions, but I suspect a deeper implementation issue prevents a trivial workaround. Correct behavior can be obtained by modifying the test in add-unless-present to "(if-not (some #(= (str (class f)) (str (class %))) coll)". I'm certain there's a better solution.

The problem is that functions currently cannot be compared for equality, only for identity. When you recompile, you have a new function object, and Hooke has no way to know if it's changed at all since your last recompile since functions are opaque. However, if you use a var as the hook instead of a function, then it has a stable value across recompiles, so equality comparisons should work.

This should probably be documented in the readme.

technomancy commented 12 years ago

Docs have been updated to reflect this; thanks.