Zen Cart Logo
Forums / Code Collaboration / observer notifier question/opinions...

observer notifier question/opinions...

Views: 2,452

Results 1 to 5 of 5
11 Dec 2021, 19:00
#1
carlwhat avatar

carlwhat

zennedOut

Join Date:
Nov 2005
Location:
los angeles
Posts:
2,953
Plugin Contributions:
8

observer notifier question/opinions...

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.

11 Dec 2021, 19:07
#2
swguy avatar

swguy

Administrator

Join Date:
Feb 2006
Location:
Tampa Bay, Florida
Posts:
10,682
Plugin Contributions:
56

Re: observer notifier question/opinions...

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.

11 Dec 2021, 19:18
#3
carlwhat avatar

carlwhat

zennedOut

Join Date:
Nov 2005
Location:
los angeles
Posts:
2,953
Plugin Contributions:
8

Re: observer notifier question/opinions...

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.

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.

11 Dec 2021, 20:17
#4
swguy avatar

swguy

Administrator

Join Date:
Feb 2006
Location:
Tampa Bay, Florida
Posts:
10,682
Plugin Contributions:
56

Re: observer notifier question/opinions...

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.

11 Dec 2021, 20:23
#5
carlwhat avatar

carlwhat

zennedOut

Join Date:
Nov 2005
Location:
los angeles
Posts:
2,953
Plugin Contributions:
8

Re: observer notifier question/opinions...

i misunderstood you.

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

thanks.