phproad / phpr-framework

[PHPR Framework] PHPRoad system files
http://phproad.com
MIT License
12 stars 2 forks source link

Phpr_Events::add_event interface #3

Closed highruned closed 7 years ago

highruned commented 11 years ago

Normal PHP works in a way that a callback is either a string (global function) or an array (class method, now static methods), eg. call_user_func_array

Thus I think the interface for Phpr::$events->add_event('service:on_extend_request_model', $this, 'extend_service_request_model'); should be changed to Phpr::$events->add_event('service:on_extend_request_model', array($this, 'extend_service_request_model'))); for consistency. Most intermediate PHP devs should know this convention and make sense to them. In addition, you can call a global funct or anything PHP callback support now or in the future, eg. 5.3 lambdas.

snetty commented 11 years ago

Agreed

Sent from my iPhone

On 19 Apr 2013, at 17:38, Eric Muyser notifications@github.com wrote:

Normal PHP works in a way that a callback is either a string (global function) or an array (Class method, now static methods), eg. call_user_func_array

Thus I think the interface for Phpr::$events->add_event('service:on_extend_request_model', $this, 'extend_service_request_model'); should be changed to Phpr::$events->add_event('service:on_extend_request_model', array($this, 'extend_service_request_model'))); for consistency. Most intermediate PHP devs should know this convention and make sense to them. In addition, you can call a global funct or anything PHP callback support now or in the future, eg. 5.3 lambdas.

— Reply to this email directly or view it on GitHub.

daftspunk commented 11 years ago

Take a look at the way Phpr_Cron works

public function subscribe_crontab() { return array('clean_promises' => array('method' => 'cron_clean_promises', 'interval' => 720)); // Daily }

This will trigger $this->cron_clean_promises() once a day.

However

public function subscribe_crontab() { return array('clean_promises' => array('method' => 'MyClass::cron_clean_promises', 'interval' => 720)); // Daily }

Will trigger MyClass::cron_clean_promises once a day.

I prefer this syntax because it's cleaner. Thoughts?

daftspunk commented 11 years ago

To elaborate, the proposed syntax I am offering is:

Phpr::$events->add_event('service:on_extend_request_model', 'extend_service_request_model');

and

Phpr::$events->add_event('service:on_extend_request_model', 'MyClass::extend_service_request_model');

One potential problem is the method must be declared as static, and I can't pass a pre-exisiting object to it. So this might be the fatal flaw to my suggestion.

daftspunk commented 11 years ago

After re reading this, I think it's better to convert everything to Eric's initial suggestion. The native php approach is best. face palm

highruned commented 7 years ago

Still love what we built here but gotta clean up my open issues~