View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 2339 | Composr | core | public | 2016-03-23 02:36 | 2016-07-15 18:45 |
| Reporter | Chris Graham | Assigned To | Chris Graham | ||
| Priority | normal | Severity | feature | ||
| Status | resolved | Resolution | fixed | ||
| Summary | 2339: Tidy up hook call pattern | ||||
| Description | We currently do like... $hooks = find_all_hooks('systems', 'SOME_HOOK_TYPE'); foreach (array_keys($hooks) as $hook) { require_code('hooks/systems/SOME_HOOK_TYPE/' . $hook); $ob = object_factory('Hook_SOME_HOOK_TYPE_' . $hook); $ob->DO_SOMETHING(); } We would instead do like... $hooks = instantiate_all_hooks('systems', 'SOME_HOOK_TYPE'); foreach ($hooks as $ob) { $ob->DO_SOMETHING(); } Implement the API for this, then apply throughout our code. Allow the old mechanism to work still though, find_all_hooks is still a useful function that instantiate_all_hooks can use. | ||||
| Tags | No tags attached. | ||||
| Attach Tags | |||||
| Time estimation (hours) | 3 | ||||
| Sponsorship open | |||||
|
|
Oh, and the new pattern should accommodate for corrupt PHP files, and use the filter_naughty function. |
|
|
I was thinking whether this is an opportunity to make all the hooks implement a class interface. From a code quality point of view it would be an improvement, with minimal code complexity cost due to it being wrappable inside the new API. However, I don't like the performance implications. For each hook type we'd need to load up another file that defines the interface, then PHP would need to check interface rules at run-time. That's actually not at all trivial - if you consider a page using 20 hook types, that's 20 extra PHP files loaded. So I think it's best to leave that. It makes more sense for compiled languages. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2016-03-23 02:36 | Chris Graham | New Issue | |
| 2016-03-23 02:37 | Chris Graham | Note Added: 0003484 | |
| 2016-03-24 13:43 | Chris Graham | Note Added: 0003493 | |
| 2016-07-15 18:45 | Chris Graham | Status | Not Assigned => Resolved |
| 2016-07-15 18:45 | Chris Graham | Resolution | open => fixed |
| 2016-07-15 18:45 | Chris Graham | Assigned To | => Chris Graham |
