Please comment: Using factory functions for component instantiation

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

Please comment: Using factory functions for component instantiation

Daniel Kinzler
MediaWiki offers several extension interfaces based on registering classes to be
used for a specific purpose, e.g. custom actions, special pages, api modules,
etc. The problem with this approach is that the signature of the constructor has
to be known to the framework, preventing us from moving
away from global state towards using proper dependency injection via the
constructor.

The alternative is to allow factory functions[1] to be registered instead of (or
in addition to) class names. This is already supported for actions and config
handlers, and I have now submitted a patch to also allow this for api modules
<https://gerrit.wikimedia.org/r/#/c/149183/>.

If this is accepted, I plan to do the same for special pages. Please have a look
and comment.


Let me give an example of why this is useful:

For example, if we want to define a new api module ApiFoo which uses a DAO
interface called FooLookup implemented by SqlFooLookup, we would have to use
global state to get the instance of SqlFooLookup:

  class ApiFoo extends ApiBase {
    public function __construct( ApiMain $main, $name ) {
      parent::__construct( $main, $name );

      $this->lookup = SqlFooLookup::singleton();
    }

    ...
  }

  ...
  $wgAPIMOdules['foo'] = 'ApiFoo';

The API module would be bound to a global singleton, which makes testing and
re-use a lot harder, and constitutes a hidden dependency. There is no way to
control what implementation FooLookup is used, and the class can't operate
without the SqlFooLookup singleton being there.

If however we control the instantiation, we can use proper dependency injection:


  class ApiFoo extends ApiBase {
    public function __construct( FooLookup $lookup, ApiMain $main, $name ) {
      parent::__construct( $main, $name );

      $this->lookup = $lookup;
    }

    ...
  }

  ...
  $wgAPIMOdules['foo'] = array(
    'class' => 'ApiFoo', // This information is still needed!
    'factory' => function( ApiMain $main, $name ) {
      $lookup = SqlFooLookup::singleton();
      return new ApiFoo( $lookup, main, $name );
    }
  );

Now, the dependency is controlled by the code that registers the API module (the
bootstrap code), ApiFoo no longer knows anything about SqlFooLookup, and can
easily be tested and re-used with different implementations of FooLookup.

Essentially it means that we have less dependencies between implementations, and
split the construction of the network of service objects from the actual logic
of the individual components.


Do you agree that this is a good approach? Do you see any problems with it?
Perhaps we can discuss this some more at Wikimania (I assume there will be an
architecture session there).


Cheers,
Daniel


[1] We could also register factory objects instead of factory functions,
following the abstract factory pattern. The main advantage of this pattern is
type safety: the factory objects can be checked against an interface, while we
have to just trust the factory functions to have the right signature. However,
even with type hinting, PHP does not do type checks on return values, so we
never know what the factory actually returns. Overall, individual factory
objects seem a lot of overhead for very little benefit. See also the discussion
on I5a5857fcfa075.

_______________________________________________
Wikitech-l mailing list
[hidden email]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l