silverstripe-scienceninjas/sapphire — comment on commit 927dbbe717be3975c3e763bd462a8b6ebbe0a035
File: security/Permission.php line 627
By sminnee:

I was thinking about more from the perspective of "there's a normal way of specifying a callback in PHP, let's use that".

Closures/inline functions aren't a code smell, they're just unusual in PHP. It's one thing to avoid doing things that make us require PHP 5.3, it's another to design the API so as to prevent the use of PHP 5.3 features in project code. ;-)

Admittedly, with `get_template_global_variables` (as it's renamed to in a later commit) being a static method, it's difficult to see a use for an instance variable. Instance variables would become more useful if template globals were passed in with D.I, as relevant instantiated objects.

We could, for instance, provide a fewer number of TemplateGlobalProviders: either one for all of the default objects, or a couple of them in natural partitions. It seems inappropriate coupling that SecurityToken needs to know about the template engine.

My thinking: let's mark the API as "experimental", so that we can get away with changing it in 3.1, or before 3.0 stable, if needs be. It's going to be a fairly hardy developer who is building their own template global providers.