WordPress.org

Make WordPress Core

Changes between Initial Version and Version 1 of Ticket #21170, comment 32


Ignore:
Timestamp:
07/15/12 15:11:37 (21 months ago)
Author:
carldanley
Comment:

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #21170, comment 32

    initial v1  
    33> The .namespace/identifier bit would only be used to identify the callback, so each "leaf" on the "branch" would have a name and we would be able to easily remove anonymous "leaves" by branch name + leaf name 
    44 
    5 So, as an example, when we use the action identifier 'actionname.namespace.other.third', I was thinking that there were several identifiers for this particular action. The action being 'actionname' and then several tiers of identifiers: 'actionname.namespace', 'actionname.namespace.other', 'actionname.namespace.other.third'. Each unique identifier tier would simply be it's own key in a flat array. Essentially, the callback passed lives in the 'actionname.namespace.other.third' array key. However, the problem I see with this is as follows:[[br]][[br]] 
    6  
    7 {{{ 
    8 addAction( 'foo.bar', callback1 ); 
    9 //Actions: 
    10 //ACTIONS[ 'foo.bar' ] = [ callback1 ]; 
    11  
    12 addAction( 'foo.bar.test', callback2 ); 
    13 //Actions: 
    14 //ACTIONS[ 'foo.bar' ] = [ callback1 ]; 
    15 //ACTIONS[ 'foo.bar.test' ] = [ callback2 ]; 
    16  
    17 addAction( 'foo.bar.test.foo', callback3 ); 
    18 //Actions: 
    19 //ACTIONS[ 'foo.bar' ] = [ callback1 ]; 
    20 //ACTIONS[ 'foo.bar.test' ] = [ callback2 ]; 
    21 //ACTIONS[ 'foo.bar.test.foo' ] = [ callback3 ]; 
    22 }}} 
    23  
    24 So when we doAction( 'foo.bar' ), we'd have to loop through each key in the flat array finding keys prefixed with 'foo.bar' and execute them. To me, this could get very heavy ( especially since we'd have to use for( var k in arr ) ) in different areas of a site with a lot of plugins installed. To combat this, I created the tree for easier access in calling the 'children' identifiers. Meaning, if we called 'foo.bar' in this example, we would know that 'foo.bar' would have an array of callbacks that needed execution and to find the children, we know the object had a property called children where 'foo.bar.test' and 'foo.bar.test.foo' lived in the same manner that their ancestor, 'foo.bar', was created ( as an object with properties like 'callbacks' and 'children' ). I really like the flat array approach, I was just hesitant because of look-up speed for doing anything. You could also make the value of each unique array key an object with 2 properties ('callbacks' and 'siblings' where 'siblings' would store an array of values that could be used to find siblings when looking up for deletion/execution)[[br]][[br]] 
     5I understand what you're saying now. The only question I have is how to we traverse the array to find simulated 'parents' and 'children'. If doAction( 'some.action' ) was called and we know there were several actions that existed such as 'some.action.a' and 'some.action.b', would we first find 'some.action' and then maybe look to a 'family' property that will indicate who the parent/children are? I don't think traversing each key in this flat array and determining who is prefixed with the correct action/identifier is the safest way, especially using for( var in ) to iterate. Thoughts? 
    256 
    267> If we create the nodes as instances of a small class, or even as simple objects, we can store the nodeType in each. 
    278 
    28 Consider the following: 
    29 {{{ 
    30 var appendHookToArray = function( arr, hookIdentifier, data ){ 
    31         var identifiers = hookIdentifier.split( '.' ); 
    32         var levels = [], identifier; 
    33          
    34         for( var i = 1, len = identifiers.length; i <= len; i++ ){ 
    35                 identifier = identifiers.slice( 0, i ).join( '.' ); 
    36                 if( arr[ identifier ] === undefined ){ 
    37                         arr[ identifier ] = []; 
    38                         if( i === len ){ 
    39                                 //just push it since we know it's a new identifier 
    40                                 arr[ identifier ].push( data ); 
    41                         } 
    42                 } 
    43                 else if( i === len ){ 
    44                         //insert the callback now 
    45                         arr[ identifier ] = insertHookByPriority( arr[ identifier ], data ); 
    46                 } 
    47         } 
    48          
    49         return arr; 
    50 }; 
    51  
    52 }}} 
    53  
    54 So then, when addAction is called all it does is this: 
    55 {{{ 
    56 SELF.addAction = function( action, callback, priority ){ 
    57         if( typeof callback !== 'function' ){ 
    58                 return SELF; 
    59         } 
    60  
    61         ACTIONS = appendHookToArray( ACTIONS, action, { 
    62                 'callback' : callback, 
    63                 'priority' : ( priority || 10 ) 
    64         } ); 
    65         return SELF; 
    66 }; 
    67 }}} 
    68  
    69 With 'ACTIONS' being a private class property ( 'FILTERS' would also exist as something separately. ) ACTIONS/FILTERS will def. need to be kept separate but I think that adding a 'nodeType' property to the object created is a little bit of an unnecessary waste, if i'm understanding correctly, meaning: I don't think it's necessary to specify nodeType of 'action' for 100+ different actions, as an example. We already know that it's an action because we have to access the 'ACTIONS' variable and, for this example, storing a 'nodeType' for each ACTION of value 'action' would mean making the array 600+ bytes heavier at run-time. I would use the same new 'object' class for both actions and filters. Thoughts? 
     9I agree in saying that we should keep the actions/filters in separate containers. I don't think it's necessary to specify nodeType of 'action' for 100+ different actions, as an example. We already know that it's an action because we have to access the 'ACTIONS' variable and, for this example, storing a 'nodeType' for each ACTION of value 'action' would mean making the array 600+ bytes heavier at run-time. I would use the same new 'object' class for both actions and filters and maybe pass the nodeType to the scope when needed. Thoughts? 
    7010 
    7111> Yeah, chaining is definitely handy. The question is whether it's better to support chaining or to return true/false on success/failure when adding or removing actions and filter.