I don't believe these bits work:

<?php
if (is_callable(array(__CLASS__ . '_hook', __FUNCTION__))) {
  eval('$hook = new ' . __CLASS__ . '_hook;');
  return $hook->{__FUNCTION__}($bnum);
}?>

I added the get_bib_item() function to the locum_client_hook class in locum-hooks.php. Stepping through in xdebug, the above php always fails. From tinyurl.com/y2ol62n, either the 2nd arg needs to be a static function, or the 1st argument needs to be an object (aka, $locum_client_hook, rather than 'locum_client_hook').

So the answer to that problem would be to construct a new locum_client_hook and pass in that object instead, but even that wouldn't work. The current locum_client class has a bunch of variables set, and when you construct a new locum_client_hook they're separate instances, so locum_client_hook is unaware of all that necessary information locum_client holds onto.

I think the solution might be the following. Your current class hierarchy goes locum->locum_client & locum->locum_client_hook. I think the hierarchy should be locum->locum_client->locum_client_hook. Then, every new locum_client; would be replaced with new locum_client_hook;. The locum_client_hook class would still be an empty shell, but since it's subclassing locum_client it gets all the information as if it weren't. User overrides a function, and that new function is used instead.

Same would go the locum & locum_server classes. Writing an explanation here to get your thoughts on the idea. I'll write a patch if you agree with this concept.

Comments

Sid_M’s picture

Sid_M commented

You're right, it doesn't work. That's because locum-hooks.php has not been loaded, and thus PHP cannot find the locum_client_hook class. The simple fix to this is to add this line at the top of locum-client.php:

require_once('locum-hooks.php');

I would put it immediately below this line:

require_once('locum.php');

Obviously, that does not address the larger issue you raise. I don't have a complete answer, but I can chip in with a couple of thoughts.

I can tell you that Alex at Your Library Site (http://drupal.org/user/78040) is working on a re-architecting of locum to address exactly these sorts of issues. I can't say I wholly understand what he is up to, but I believe that part of it is implementing a factory pattern which will provide a lot more flexibility in terms of what class of object gets returned when one calls new locum_client(). Assuming that he successfully implements this, I think we will come to see the current hooks approach as an interim step which was replaced by the new system.

Given that, I think it makes sense not to tinker more than necessary with the current hooks system. However, if we need to tinker, it would be better not to focus on which class locum hooks should inherit from. Instead, it would make more sense to focus on implementing locum hooks in terms of composition. Going this route would mean that the hooks class would not inherit from any locum class, and possibly not from any class at all. Upon instantiating a locum hooks object, locum client would pass a reference to itself into the hooks object, and the hooks object would store that reference in its own local variable: e.g. $this->locum. This would allow the hooks object to call locum client methods by doing something like $this->locum->some_method(). It would also allow the hooks object to access any public locum client data by doing something like $this ->locum->locum_config['some_key']. I think that would be a better way of addressing your concerns than making locum hooks inherit from locum client.

However, I continue to think that we are better off waiting for the factory architecture unless there is some pressing need which cannot wait.

In that regard, let me reassure you a bit. While it is true that locum hooks cannot call methods of locum client, it does have the same data available to it. Although this might change in future, currently locum client does not have any data which is being manipulated in the course of a page load (obviously, excluding local variables within methods which are not relevant to this conversation). All of locum client's object data is configuration data, and that gets set for every locum object in the locum constructor. Since locum hooks inherit from locum, it has exactly the same configuration data as does locum client.