observer notifier question/opinions...

[carlwhat]

i am curious about opinions. in the past, i have stuck to the docs, with regards to updating values using an observer, ie $parm1 is read only and 2-9 are update-able. see:

https://docs.zen-cart.com/dev/code/notifiers/#update

so if one were to look at this observer:

$zco_notifier->notify('NOTIFY_SEARCH_ORDERBY_STRING', $listing_sql); 

the $listing_sql is NOT update-able as it is in $p1 position. however, if we were to add the $listing_sql to the global line in the update function, we can now change it, ie:

      public function update(&$class, $eventID, &$p1, &$p2, &$p3, &$p4)
        {
            global $listing_sql;
            switch ($eventID) {
                case 'NOTIFY_SEARCH_ORDERBY_STRING':
                    $listing_sql = 'whatever the bloody heck i like';
                    break;
            }
        } 

i am curious what the downside may be on this approach as well as whether it is worthy of including in the docs.

best.

[swguy]

The downside is the usual downside of using global variables; you create code that is much harder to debug and less self-documenting.

If you feel that it's important to be able to change the first parameter, the better approach (which has been used in the past) is to simply update

$zco_notifier->notify('NOTIFY_SEARCH_ORDERBY_STRING', $listing_sql);

to

$zco_notifier->notify('NOTIFY_SEARCH_ORDERBY_STRING', $listing_sql, $listing_sql);

This is upwardly compatible and the intent is clear.

[carlwhat]

The downside is the usual downside of using global variables; you create code that is much harder to debug and less self-documenting.

If you feel that it's important to be able to change the first parameter, the better approach (which has been used in the past) is to simply update

$zco_notifier->notify('NOTIFY_SEARCH_ORDERBY_STRING', $listing_sql);

to

$zco_notifier->notify('NOTIFY_SEARCH_ORDERBY_STRING', $listing_sql, $listing_sql);

This is upwardly compatible and the intent is clear.

i am not a fan of this idea. the whole point is to not modify core code. changing the core code, i might as well as change the listing sql on that section of the code.

[swguy]

I'm suggesting you do this as a PR for inclusion into a future release. Like I said, this has been done in the past by people who needed exactly what you need.

Here's an example:

Zen Cart 1.5.6:
$zco_notifier->notify('NOTIFY_EMAIL_BEFORE_PROCESS_ATTACHMENTS', array('attachments'=>$attachments_list, 'module'=>$module));

Zen Cart 1.5.7:
$zco_notifier->notify('NOTIFY_EMAIL_BEFORE_PROCESS_ATTACHMENTS', array('attachments'=>$attachments_list, 'module'=>$module), $mail, $attachments_list);

Someone needed to be able to modify $attachments_list.

[carlwhat]

i misunderstood you.

when and if PRs start getting merged again, perhaps i will create one.

thanks.