+1 del $op. Couldn't this also lead to some deeper integration with actions/triggers and allow people to write their own $op states as functions? Regardless I think it's a logical progression instead of everyone writing a switch statement to parse out which $op it is everytime. On Fri, May 9, 2008 at 8:35 AM, Moshe Weitzman <weitzman@tejasa.com> wrote:

I agree that we should move away from op to dedicated hooks. for example, hook_nodeapi_delete().

Hook_help can be should be shot and killed by advanced_help module.

On Fri, May 9, 2008 at 5:23 AM, Gábor Hojtsy <gabor@hojtsy.hu> wrote:

...

Hello,

Now that the registry patch landed, it looks like the time of $op might be over. General hooks implementing different branches with $op will always be loaded regardless of the exact need for them. Larry already suggests using the more granular *_preprocess_hookname() type of callbacks instead of the generic *_preprocess() callbacks for theme stuff, so that only those needed will be loaded. Take nodeapi for example. Code for loading nodes might often be needed, but not code for updating or deleting them, right? Those could easily be admin.inc stuff.

So what do you think about $op? Should we evaluate on a case-by-case basis to make function level callbacks out of $op's or just do it wholesale (no, I am not volunteering for a patch) to get an overall consistent API.

Gabor

On Friday 09 May 2008, Daniel F. Kudwien wrote:

...

Regardless I think it's a logical progression instead of everyone writing a switch statement to parse out which $op it is everytime.

I agree, but I guess this will make debugging a bit harder.

On the contrary, smaller, more targeted functions should make debugging easier, as well as unit testing easier. You then have more bite-sized code snippets that have fewer possible states and fewer internal logical branches, hence they're more deterministic and easier to debug. You can also move them about to different files if needed, which helps the registry. The only downside is if you need to share data from one op to another (say from submit to validate to update). Right now that is trivially easy with a static variable. If they're in separate functions, you need a collector function of some kind or a variable_set(), or a central static variable store (chx submitted a patch for that which I've not looked into yet, but I think that should be OO rather than purely procedural to make it more modular, for which he will probably try to smack me), or some other mechanism. (Global variables for that are a design flaw; please don't try it.) It's a solvable problem. Similarly, most form_alter implementations can easily be made into form_$form_id_alter functions, which is a nice simplification and a registry-help for the same reasons. I have plans percolating in the back of my head to modernize hook_block, took, to make it more menu-ish in structure and break it out into multiple functions. That's not something I'm going to spend any code time on until after the database work is completed, though. I have an irrational hatred of switch statements, so eliminating hooks that are nothing but nested switch statements half the time gets a big +1 from me. :-) -- Larry Garfield AIM: LOLG42 larry@garfieldtech.com ICQ: 6817012 "If nature has made any one thing less susceptible than all others of exclusive property, it is the action of the thinking power called an idea, which an individual may exclusively possess as long as he keeps it to himself; but the moment it is divulged, it forces itself into the possession of every one, and the receiver cannot dispossess himself of it." -- Thomas Jefferson

On Tue, May 13, 2008 at 5:40 AM, Larry Garfield wrote:

I have plans percolating in the back of my head to modernize hook_block, took, to make it more menu-ish in structure and break it out into multiple functions. That's not something I'm going to spend any code time on until after the database work is completed, though.

Looks like dropcube had the same idea: http://drupal.org/node/257032

On 13 May 2008, at 6:40 AM, Larry Garfield wrote:

I have an irrational hatred of switch statements, so eliminating hooks that are nothing but nested switch statements half the time gets a big +1 from me. :-)

the very first iteration of fapi was to have each form be one function with a switch statement. can you imagine how nasty that would have been ? i'm _firmly_ in the multiple function camp. I even write little stubs for my own nodeapi functions, that call modulename_nodeapi_$op functions etc.

api.drupal.org and the handbooks. If it's not sufficiently clear from those, that is a documentation bug. Please file a patch. :-) --Larry Garfield On Tue, 13 May 2008 11:38:49 -0400, "Jerad Bitner" <sirkitree@gmail.com> wrote:

This is great, I understand the pros of this approach, however, when first learning about hooks and $op, it was nice to be able to dsm($op) and find out what all was available to me. How would a newer dev find this information now? In the registry?

On Tue, May 13, 2008 at 11:34 AM, Adrian Rossouw <adrian@bryght.com> wrote:

...

On 13 May 2008, at 6:40 AM, Larry Garfield wrote:

...

I have an irrational hatred of switch statements, so eliminating hooks that are nothing but nested switch statements half the time gets a big +1 from me. :-)

the very first iteration of fapi was to have each form be one function with a switch statement.

can you imagine how nasty that would have been ?

i'm _firmly_ in the multiple function camp. I even write little stubs

for

...

my own nodeapi functions, that call modulename_nodeapi_$op functions etc.

Nancy Wichmann wrote:

Larry Garfield wrote: "api.drupal.org and the handbooks. If it's not sufficiently clear from those, that is a documentation bug. Please file a patch."

Better yet join the Documentation team, get CVS access, and fix it yourself.

Since this isn't even a feature yet, it's a bit early to point people at docs and say 'go fix it'. I wonder if maybe we, as a community, rely on that answer a little too much.

litwol is already writing the de-$op patch, it's a must, there is nothing to debate, move over, bring on the next bikeshead thread.

As a new and unexperienced developer of drupal modules I must say I am all for this suggestion. These $op-variables is quite a pain to keep track off. I also think that much of my (and others) code will be much cleaner and easier to read and understand if there where hooks instead of an $op variable (which in many cases creates a huge switch statement, which is not pretty). On Sat, May 10, 2008 at 1:08 AM, James Walker <walkah@walkah.net> wrote:

Having just finished a workshop where we have to explain this to new drupal developers, I'd like to echo David's sentiment - having $op - specific parameters is *very* confusing.

On 9-May-08, at 10:50 AM, David Timothy Strauss wrote:

...

Please get rid of $op at least in the cases where the arguments change. I want to see an end to $arg1, etc.

----- "Gábor Hojtsy" <gabor@hojtsy.hu> wrote:

...

Hello,

Now that the registry patch landed, it looks like the time of $op might be over. General hooks implementing different branches with $op will always be loaded regardless of the exact need for them. Larry already suggests using the more granular *_preprocess_hookname() type of callbacks instead of the generic *_preprocess() callbacks for theme stuff, so that only those needed will be loaded. Take nodeapi for example. Code for loading nodes might often be needed, but not code for updating or deleting them, right? Those could easily be admin.inc stuff.

So what do you think about $op? Should we evaluate on a case-by-case basis to make function level callbacks out of $op's or just do it wholesale (no, I am not volunteering for a patch) to get an overall consistent API.

Gabor