Injector + static methods.

[Patrick Nelson]

Quick question out of curiosity (plus it should be useful): Is there currently a set method for calling static methods on injected classes? 

For example -- right now error page responses are essentially generated out of the static method ErrorPage::response_for($errorCode) method, apparently (after a quick debug session and a stack trace) via ModalAsController->getNestedController(). So let's say I want to speed up page load times for 404 errors and opt for the static version, I could write a custom variation of the method ::response_for() which contains the static file logic ahead of the database-driven version. The problem here is that because these methods are called statically, I have no way to inject my custom logic without modifying core.

Is there a good way around this if we were to change the way core code calls static methods? Or, if this method of calling static methods exists (by proxy of Injector), how is that done so that I can patch this particular part of core for my purposes?

[Ingo Schommer]

Congratulations, you've identified the primary argument against static methods ;) 

You can call static methods on instances (at least in the PHP 5.6 I've tested with): 

$obj = Injector::inst()->get('ErrorPage');

$obj::response_for(404);

I can't find that as documented/supported behaviour anywhere in the PHP documentation though:

http://php.net/manual/en/language.oop5.references.php

http://php.net/manual/en/language.oop5.static.php

http://php.net/manual/en/language.oop5.paamayim-nekudotayim.php

So not too keen to adopt that as standard core practice.

Other than Injector::inst() and a few ORM shortcuts like DataList::*(),

I think we need to reduce our usage of static methods in core.

In terms of backwards compatibility, we could phase out static method use

by using __callStatic(), and only allow aliasing similar to Laravel Facades. 

In your case, I would suggest creating an ErrorPage->responseFor() instance method,

with an extend() invocation to allow customisation. Dependency injection doesn't seem

the right level of granularity for this customisation.

--
You received this message because you are subscribed to the Google Groups "SilverStripe Core Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to silverstripe-d...@googlegroups.com.
To post to this group, send email to silverst...@googlegroups.com.
Visit this group at https://groups.google.com/group/silverstripe-dev.
For more options, visit https://groups.google.com/d/optout.

--

Ingo Schommer | Solutions Architect
SilverStripe (http://silverstripe.com)
Mobile: 0221601782
Skype: chillu23

[Patrick Nelson]

I agree. And those static methods _are everywhere_. It may spawn a larger discussion on how to better emulate The Laravel Way(tm) for SS4 or even SS5 (depending on the effort required). That is, to implement a legit IoC container (now "service container"). Personally I'd love a short term solution by proxy of the Injector, but yeah leaving the method as static and then calling it via instance style -> is not a good idea (especially since doing so should issue warnings if you have the strict error level enabled). A better solution may be to convert the methods to instance level by removing the static keyword (obvious an API breaking change) and then start using the injector to instantiate and call it like normal. 

So, since either approach would potentially be a lot of work and I assume would have to be implemented in SS4 anyway (given the API changes), maybe there should be a discussion (or RFC?) centered on implementing some sort of service provider/container/IoC abstraction? There are plenty of situations where a singleton instance (or global style) call would be performed from within core that a user may wish to override and not simply "->extend()". In my particular circumstance, I needed the ability to completely override the method instead of just extend, since I would need the ability to optionally return a $response ahead of time before the standard/default process proceeds (in some situations). So for me, since this is statically called, a facade/IoC call would have been perfect and easily implemented.

Situations like these have made me want to scrap it all and just re-implement the awesomeness of SilverStripe on a framework like Laravel, but that may a be sacrilege thing to say (albeit I know devs here from time to time will cite practices they've seen/learned in Laravel to help grow SS).

[Ingo Schommer]

> That is, to implement a legit IoC container (now "service container")

How do you see this different from the Injector system we already have? To me that's a "service container".

It's just a matter of using it more deeply in legacy core systems which weren't designed for it (static methods).

There's around 700 public static methods in framework, at a guess about a third of them is already deprecated in favour of the Config API.

A good portion of them are de-facto facades already (Cookie, Requirements, Session, Config).

Maybe as a starting point, you could go through the remaining static core methods and prioritise the ones you think are likely to require frequent customisation, such as ErrorPage::response_for()? As a first step, we could simply make them instance methods and pass through static calls. As a second step, we should think about more structured aliases with less boilerplate (where class_alias() and __callStatic() metaprogramming will come in handy, see http://alanstorm.com/laravel_facades).