[wp-trac] [WordPress Trac] #37699: Death to Globals Episode #1: A Registry, A Pattern

[WordPress Trac]

#37699: Death to Globals Episode #1: A Registry, A Pattern
----------------------------+------------------
 Reporter:  wonderboymusic  |       Owner:
     Type:  enhancement     |      Status:  new
 Priority:  normal          |   Milestone:  4.7
Component:  General         |     Version:
 Severity:  normal          |  Resolution:
 Keywords:                  |     Focuses:
----------------------------+------------------

Comment (by jacobsantos):

 I think your design is too "WordPress", in that it condenses what should
 be multiple methods or classes to a single method. I would also suggest
 The PHP League Container library. The problem with Pimple or really any
 container library is that it still needs to be accessed in some way.
 Something you mention below. Whether that retrieval is in WP or some other
 Singleton, that class would not be a container. It would hold the
 container.

 I don't believe you looked at MimicCMS Storage implementation. As a POC, I
 would rather you used it as a foundation since the ideas are:
  1. The storage container should be separate from the retrieval. This
 should allow for proper testing of each part of the service container
 without interference of other pieces of WordPress code. Proper unit test
 cases, instead of the herculean effort found currently in the WordPress
 test suite to make the test cases as unit as possible. It is easier to
 unit test something that is meant to be independent than to make something
 that is not meant to be independent unit testable.
  1. The focus of the service container implementation should be separable
 from WordPress. This means that you would have another service container
 implementation in the wild that could gain traction outside of WordPress
 and therefore support larger than the maintainers. I doubt this would
 happen as there would likely be a split between what is included in
 WordPress and the progress of any library. The other, more important,
 reason is so that the implementation in WordPress could be replaced with
 another library at a later date. Think WP_HTTP with Requests, except
 WP_Service with Pimple or League Container.

 The reason I don't recommend Pimple is that array access is a bit more
 difficult to work with than object methods.

 {{{#!php
 <?php
 $service = WP::service();
 $wpdb = $service['wpdb'];


 /// ---------------

 $service = WP::service();
 $service['wpdb'] = $service->protect(function() {
     return new wpdb;
 });
 $service['event'] = function() {
     return new WP_Event;
 };
 }}}

 Compare that to League's Container library

 {{{#!php
 <?php
 WP::service()->share('wpdb', new wpdb());
 WP::service()->get('wpdb');
 }}}

 It seems the primary advantage and the reason Pimple is being chosen is
 because it is simple with few lines of code. Practically, the only thing
 you would need to do to make it PHP5.2 is to remove the namespace and
 replace the acceptance of closures. If you want to remove SPL dependency
 and you should do this anyway, you would just rename the ArrayAccess
 methods.

 I will say that the protection and service layer of MimicCMS Storage is
 within the classes you pass. It is a better candidate for PHP7 with
 anonymous classes, but I think it would better serve you to imitate the
 composition and design of it than Pimple. The PHP League Container library
 may seem more involved, but once you remove the interfaces and drill down
 to its code, it isn't any more or less complex than MimicCMS Storage.

 == What is This? ==


 Replying to [comment:73 ChriCo]:
 > {{{#!php
 > <?php
 > $c = new WP_Service_Container();
 > $c->protected( 'foo', 'bar' );
 > // ---------------
 > // protected
 > $c->protected( 'foo', 'bar' );
 > $c->set( 'foo', 'b..arrrrr i\'m a pirate' ); // '''not''' possible -
 exception or at least return FALSE;
 > echo $c->get( 'foo' ); // bar
 >
 > // factory
 > // just an example, since we're not able to use closures we maybe need
 to build something around it.
 > // $key, $class_name (instance of class name is the returned value),
 $args
 > $c->factory( 'wp.db', 'wpdb', array( 'db-user', 'db-pass', 'db-name',
 'db-host' ) );
 > $wpdb1 = $c->get( 'wp.db' ); // complete new instance of wpdb
 > $wpdb2 = $c->get( 'wp.db' ); // complete new instance of wpdb
 > }}}

 Again, the design seems to accept arbitrary data types. This is unwise and
 I would recommend instead creating a class to contain the data types that
 are not objects.

 Prefer instead:

 {{{#!php
 <?php

 final class Storage_Factory_Wpdb implements Storage_FactoryInterface {
     // Implementation creates and stores wpdb instance.
 }

 final class Storage_Data implements Storage_DataInterface {
     private $data;

     public function __construct($data) {
         $this->data = $data;
     }

     public function get() {
         return $this->data;
     }

     public function set($data) {
         $this->data = $data;
     }
 }

 final class Storage_ProtectedData implements Storage_DataInterface {
     private $data;

     public function __construct($data) {
         $this->data = $data;
     }

     public function get() {
         return $this->data;
     }

     public function set($data) { }
 }

 $c = new WP_Service_Container();

 $c->protected( 'foo', new Storage_Data('bar') );

 // ---------------
 // protected
 $c->protected( 'foo', new Storage_Data('bar') );
 $c->set( 'foo', new Storage_Data("b..arrrrr i'm a pirate") ); // '''not'''
 possible - exception or at least return FALSE;
 echo $c->get( 'foo' )->get(); // bar

 // factory
 // just an example, since we're not able to use closures we maybe need to
 build something around it.
 // $key, $class_name (instance of class name is the returned value), $args
 $c->factory( 'wp.db', new Storage_Factory_Wpdb(array( 'db-user', 'db-
 pass', 'db-name', 'db-host' )) );
 $wpdb1 = $c->get( 'wp.db' ); // complete new instance of wpdb
 $wpdb2 = $c->get( 'wp.db' ); // complete new instance of wpdb

 }}}

 The concept is more that if you always return an object, then you can do
 more with it. Take a facade for example.

 {{{#!php
 <?php
 class Facade_WP_Database extends WP_Facade {
     protected function _serviceName() {
         return 'wp.db';
     }
 }

 Facade_WP_Database::insert( 'table', array( 'fieldName' => 'data' ) );
 }}}

 {{{#!php
 <?php
 class Facade_Foo extends WP_Facade {
     protected function _serviceName() {
         return 'foo';
     }
 }

 echo Facade_Foo::get(); // bar -- from above.
 }}}

 Sure, it borrows much from Laravel, but it is simple enough to implement
 with a service container. The point is that it doesn't matter what the
 implementation for registering and retrieving from the service container,
 because no one is going to use it. Or more that there is a better, easier
 way to use it.

 Tell people that there is an easier API with a Facade and see if people
 move over to it. Laravel is MIT licensed, you would just have to backport
 and provide attribution, but given that the implementation is basic and
 simple, it shouldn't be that difficult to implement, if you don't want to
 look at Laravel.


 That it is more verbose is a nature of supporting a PHP version that
 doesn't support many of the features in PHP5.3 and above. Since it is just
 objects and closures are objects, it should be fine to store closures and
 execute them when retrieved.

 It also allows for more composition with the code, which should open the
 possibilities and extensions a lot more than a simple array would.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/37699#comment:74>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform