Skip to main content ↑ ↓
Topic: Yikes! Must be more careful with hook names! (Read 3278 times) previous topic - next topic
0 Members and 1 Guest are viewing this topic.

Yikes! Must be more careful with hook names!

So I'm trying to hook  call_integration_hook('integrate_register_after', array($regOptions, $memberID)); inside registerMember.   Only my hook keeps getting called for any register action, even showing the registration agreement.  So I do a backtrace and see my function is getting called by Dispatcher.class.php line 331?

What is this I ask?  Why it's this.

Code: [Select]
      call_integration_hook('integrate_' . $hook . '_after', array($this->_function_name));
And that translates too, integrate_register_after which is the same name as the hook called inside registerMember, only now it's called anytime action=register is called.

I wonder if this happens anywhere else?

Maybe rename the one in dispatch to a suffix of 'after_dispatch' instead of just 'after' ?  At least this makes it unlikely to conflict with your general naming scheme.
Last Edit: March 14, 2014, 04:03:46 am by scripple

Re: Yikes! Must be more careful with hook names!

Reply #1 –

Arg!!!
Yeah, at the beginning there was "action" around in the name (because the hook was in fact targetting the sub-action), then I changed my mind, but I didn't consider possible conflicts.
I'd say integrate_action_XXX_after and integrate_action_XXX_before so that it reflects the fact that is for the "action".

Thank you very much scripple!! :D
Bugs creator.
Features destroyer.
Template killer.

Re: Yikes! Must be more careful with hook names!

Reply #2 –

Fixed here:
https://github.com/emanuele45/Dialogo/commit/692ce078dc114123cbb6e7ee2ca1f0f43ae1e989

There are few other potentially problematic "generic" hooks, I would do that:
https://github.com/emanuele45/Dialogo/commit/3c7c6d121285244d6a182b0fd6cfc0ba027790e2
What do you think?
Also @Spuds and @TE
Bugs creator.
Features destroyer.
Template killer.

Re: Yikes! Must be more careful with hook names!

Reply #3 –

Makes sense  :D , its for the same reasons we updated most of the admin controllers to use the modify and save names, for  consistency and to hopefully avoid conflicts.

Re: Yikes! Must be more careful with hook names!

Reply #4 –

I kind of liked having dispatch in the name as I'd know where to find it.  But as long as the conflicts are cleared I'm ok with whatever naming scheme you go with.

I do appreciate everyone's responses to these bug reports.  Thank you.

Re: Yikes! Must be more careful with hook names!

Reply #5 –

If you (all) prefer dispatch it can be changed, I used action because it reflects more what the hook is about (the action) and not the place the hook is called (the dispatcher).
For example, tomorrow the dispatcher may be renamed to router, but the hooks would stay the same because what they do is let you hook the action. Of course rename action would be tricky. Well, hypothetical situations, I don't think we are going to rename dispatcher any time soon, so not very useful.

That said: Dispatch deals with actions, while Action deals with sub-actions. I never really liked that because it leads to confusion IMHO.
Probably Action was better named as Area_Dispatch or something like this.
Dunno if it is worth changing now, also because keep backward compatibility is pretty easy in that case, it's just a matter of changing the class to a wrapper, so it could be changed even in 1.1.
Bugs creator.
Features destroyer.
Template killer.

Re: Yikes! Must be more careful with hook names!

Reply #6 –

I really am fine with whatever naming scheme you prefer as long as it's conflict free.  It was just an opinion.  :)

Re: Yikes! Must be more careful with hook names!

Reply #7 –

Quote from: Spuds – Makes sense  :D 
+1
Thorsten "TE" Eurich
------------------------