Re: [PHP-DEV] A modest proposal: __contructStatic
Kris Craig wrote on 16/02/2015 01:40: create the static instance Isn't that essentially a contradiction in terms? I can't help but feel that blurring the line between static and non-static classes/methods would cause more harm than good. I've never really done any work with Redis before so I'm also having trouble understanding the use case for this given that everybody's talking about this solely in the context of Redis. It's basically like any database connection - in a simple app, you just need a single connection that you can run lots of queries against, so global/singleton access is very convenient. I do see the use case, but am not sure it's compelling enough to add a language feature, given that a similar effect can be achieved using existing features. Regards, -- Rowan Collins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] A modest proposal: __contructStatic
Am 15.02.2015 21:05 schrieb Rowan Collins rowan.coll...@gmail.com: This sounds to me like you should just be using the Singleton pattern, Of course this is singleton under the hood. // Now wherever in the code you want the default instance, just use this: $value = MyRedis::singleton()-get($key); You can surely see how this is more readable / easier to write: $value = MyRedir::get($key); The nice thing about an explicit Singleton is you can migrate to Dependency Injection (call $redis = MyRedis::singleton() and start passing the instance around, I _can_ do that with my redis class, where I need it, by calling a method MyRedis::link() which just returns $this. But the places where I need that are rare. ... or to a cleverer factory / service locator (store more than one instance in different static variables or a keyed array, to connect to different stores for different purposes). This i do with subclasses, where the subclass define suitable static protected properties that guide the connect logic (which sits in the baseclass). I have a global redis, a local redis (local to the host the code runs on), a volatile redis (global, but does not make prersistent dumps), and so on. Calling code ready red_global::this(), red_volatile::that(), and so on. You don't get it better readable (and greppable) as that. Making static calls implicitly access a singleton seems like it would make code harder to read, and harder to extend, I don't understand. It's the best to read to me. And the best to extend - I just make another subclass. Of course, there are places (in other classes) where stuff needs to use such a redis instance without caring which one it is - i.e. an instances is passed to a constructor there - in which case I can use the MyRedis::link() method to get at the singleton instance. But in most (I'd guess 90%) of the calling places, it's just the static calls I need, and writing each of them as MyRedis::singleton()-xxx() is more tedious. best regards Patrick
Re: [PHP-DEV] A modest proposal: __contructStatic
2015-02-15 21:04 GMT+02:00 Patrick Schaaf p...@bof.de: Hello Internals, seeing the static calling of instance methods being discussed again, I want to ask whether the following idea would maybe have a chance? In our codebase we have one set of classes where it was very useful to be able to call the same methods both statically and nonstatically: a set of wrapper classes over phpredis where subclasses know which set of redis servers to talk to (plus other config like retry strategies and timeouts). In most cases calling code does not need to be concerned with these details and just calls methods like red_global::get() or red_local::put(). By neccessity the implementation of this class set, must make use of both __call() and __callStatic() magic methods, with both then dispatching to a delegate phpredis instance, and in the case of __callStatic(), making up-front a singleton like new self once. For a set of additional helper methods (not present on the underlying phpredis) I additionally have a special mapping array of real method names to internal implementation names. A similar approach can be useful for database connection abstraction classes. Now for the idea how this might be made much more simple: onviously in the light of the static call to method using $this warning/detection, PHP already knows exactly where it hits such a static call to an instance method. At that point, check whether the class has a static instance already. When it has one, call the method with $this being that static instance. Otherwise, create the static instance, calling a new magick method named __constructStatic() to initialize it. Only classes that have such a __constructStatic() method, would be treated that way. For other classes the static call to instance method case would be an error as before/discussed. This approach would clearly simplify my redis wrapper classes, maybe even enable them to be proper subclasses of phpredis (saving an indirection to a $this-delegate object). All the special dispatch and __callStatic instantiation logic could go away. The cost would be one internal pointer (to the static instance object) per class, a kind of hidden static protected property, and only needed when the class (or one of its superclasses) _has_ a __constructStatic method in the first place. What do you all think? Interesting? Somebody hot for helping with impementation? Then I'd make an RFC of it. best regards Patrick +1 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] A modest proposal: __contructStatic
On 15/02/2015 19:04, Patrick Schaaf wrote:
In our codebase we have one set of classes where it was very useful to be
able to call the same methods both statically and nonstatically: a set of
wrapper classes over phpredis where subclasses know which set of redis
servers to talk to (plus other config like retry strategies and timeouts).
In most cases calling code does not need to be concerned with these details
and just calls methods like red_global::get() or red_local::put().
By neccessity the implementation of this class set, must make use of both
__call() and __callStatic() magic methods, with both then dispatching to a
delegate phpredis instance, and in the case of __callStatic(), making
up-front a singleton like new self once. For a set of additional helper
methods (not present on the underlying phpredis) I additionally have a
special mapping array of real method names to internal implementation names.
This sounds to me like you should just be using the Singleton pattern,
at which point you need no magic calls at all. It can even be done as a
subclass of an existing non-static class:
class MyRedis extends Redis {
private static $singleton;
public static function singleton() {
if ( is_null(self::$singleton) ) {
self::$singleton = new self; // may need parameters here
}
return self::$singleton;
}
// Other methods can go here, just like a normal subclass
}
// Now wherever in the code you want the default instance, just use this:
$value = MyRedis::singleton()-get($key);
MyRedis::singleton()-someCustomAction($anotherKey);
The nice thing about an explicit Singleton is you can migrate to
Dependency Injection (call $redis = MyRedis::singleton() and start
passing the instance around, until you get to the point where it is only
called once), or to a cleverer factory / service locator (store more
than one instance in different static variables or a keyed array, to
connect to different stores for different purposes).
Making static calls implicitly access a singleton seems like it would
make code harder to read, and harder to extend, in exchange for typing a
few characters on each call (you could call it something shorter, like
::inst() if you valued brevity) so a -1 from me.
Regards,
--
Rowan Collins
[IMSoP]
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] A modest proposal: __contructStatic
On 15/02/2015 20:20, Patrick Schaaf wrote:
Am 15.02.2015 21:05 schrieb Rowan Collins rowan.coll...@gmail.com
mailto:rowan.coll...@gmail.com:
This sounds to me like you should just be using the Singleton pattern,
Of course this is singleton under the hood.
// Now wherever in the code you want the default instance, just use this:
$value = MyRedis::singleton()-get($key);
You can surely see how this is more readable / easier to write:
$value = MyRedir::get($key);
Actually, no, I find that harder to read accurately - there is no clue
there that there is actually a singleton under the hood, and that this
is actually an instance method in disguise.
For writing, it saves, in this case, 13 characters; change the method
name to inst(), and that difference goes down to 8 characters.
And if you use it lots of times in succession, you wouldn't even need to
call the accessor every time:
$redis = MyRedis::inst();
$foo = $redis-get('foo');
$bar = $redis-get('bar');
// etc
In that scenario, you've had to write one extra line at the top of the
method, and the rest of the lines are probably shorter (if the variable
name is more than one character shorter than the class name). Plus, you
have the bonus of being able to use an injected connection later.
The nice thing about an explicit Singleton is you can migrate to Dependency Injection (call $redis =
MyRedis::singleton() and start passing the instance around,
I _can_ do that with my redis class, where I need it, by calling a
method MyRedis::link() which just returns $this. But the places where
I need that are rare.
You may be able to do it with your current implementation, but how do
you propose to do it with an invisible singleton property implicitly
created by __constructStatic?
... or to a cleverer factory / service locator (store more than one instance in different static variables or a
keyed array, to connect to different stores for different purposes).
This i do with subclasses, where the subclass define suitable static
protected properties that guide the connect logic (which sits in the
baseclass). I have a global redis, a local redis (local to the host
the code runs on), a volatile redis (global, but does not make
prersistent dumps), and so on. Calling code ready red_global::this(),
red_volatile::that(), and so on. You don't get it better readable (and
greppable) as that.
OK, but you need a new subclass every time, not just a new method, or
new parameter; that limits your options. For instance, you can't
dynamically create instances based on configuration of a reusable
library, which could be as simple as adding an optional parameter to a
standard Singleton:
class MyRedis {
private static $instances;
public static function inst($type='default') {
global $config;
if ( ! array_key_exists(self::$instances, $type) ) {
self::$instances[$type] = new self(...$config['redis_options'][$type]);
}
return self::$instances[$type];
}
}
Any function that already uses the $redis = MyRedis::inst(); pattern
can then quickly be changed to $redis = MyRedis::inst('volatile'); and
continue to work fine.
Making static calls implicitly access a singleton seems like it would make code harder to read, and harder to
extend,
I don't understand. It's the best to read to me.
OK, we'll call that one a matter of opinion.
And the best to extend - I just make another subclass.
Extend was perhaps not the word I should have chosen; I was thinking
of extending the use of the class beyond the current coding pattern.
By implementing one specific use case as a magic method, you make it
harder to do anything other than that one use case; or, you end up
creating a whole set of magic methods and features, which are then no
easier than just implementing the whole thing by hand.
I deliberately didn't go into details of the arguments against the
Singleton pattern in general in my last e-mail, because I was comparing
your magic code to a normal Singleton. However, those criticisms are
out there, and adding a feature to the language specifically in order to
support Singletons, in a very limited way, will be considered by some as
encouraging bad practice.
Regards,
--
Rowan Collins
[IMSoP]
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] A modest proposal: __contructStatic
On 15/02/2015 19:04, Patrick Schaaf wrote:
By neccessity the implementation of this class set, must make use of both
__call() and __callStatic() magic methods, with both then dispatching to a
delegate phpredis instance, and in the case of __callStatic(), making
up-front a singleton like new self once. For a set of additional helper
methods (not present on the underlying phpredis) I additionally have a
special mapping array of real method names to internal implementation names.
A quick thought - if you want to stick with the magic static call
pattern, you can implement this much more simply by doing something
similar to Laravel's facades [1]:
class MyRedis extends Redis {
// extra instance methods here
}
class MyRedisFacade {
private static $instance;
public static function __callStatic($method, $params) {
if ( is_null(self::$instance) ) {
self::$instance = new MyRedis;
}
self::$instance-$method(...$params);
}
}
This basically implements in userspace what you propose to add to the
language, with the only caveat being that you must separate the concerns
of adding extra functionality, and wrapping it in a static facade, into
two separate classes.
[1] http://laravel.com/docs/4.2/facades
--
Rowan Collins
[IMSoP]
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] A modest proposal: __contructStatic
Patrick Schaaf wrote: Am 15.02.2015 21:05 schrieb Rowan Collins rowan.coll...@gmail.com: // Now wherever in the code you want the default instance, just use this: $value = MyRedis::singleton()-get($key); You can surely see how this is more readable / easier to write: $value = MyRedir::get($key); It's possible to add static methods or use __callStatic() to delegate to the singleton's instance methods, though. -- Christoph M. Becker -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] A modest proposal: __contructStatic
create the static instance Isn't that essentially a contradiction in terms? I can't help but feel that blurring the line between static and non-static classes/methods would cause more harm than good. I've never really done any work with Redis before so I'm also having trouble understanding the use case for this given that everybody's talking about this solely in the context of Redis. --Kris
Re: [PHP-DEV] A modest proposal: __contructStatic
Am 15.02.2015 23:34 schrieb Rowan Collins rowan.coll...@gmail.com:
You can surely see how this is more readable / easier to write:
$value = MyRedir::get($key);
Actually, no, I find that harder to read accurately - there is no clue
there that there is actually a singleton under the hood, and that this is
actually an instance method in disguise.
Fair enough. You judge readabilidy by an unaccustomed reader can grasp
(better or worse) what lies behind this construct. I have a different
focus, as I'm just a herder of a small group of internal developers.
The nice thing about an explicit Singleton is you can migrate to
Dependency Injection (call $redis = MyRedis::singleton() and start
passing the instance around,
I _can_ do that with my redis class, where I need it, by calling a
method MyRedis::link() which just returns $this. But the places where I
need that are rare.
You may be able to do it with your current implementation, but how do you
propose to do it with an invisible singleton property implicitly created by
__constructStatic?
Without any caller side change, calling this already existing instance
method:
public function link() { return $this; }
OK, but you need a new subclass every time, not just a new method, or new
parameter; that limits your options. For instance, you can't dynamically
create instances based on configuration of a reusable library, which could
be as simple as adding an optional parameter to a standard Singleton:
Nothing would stop me from writing and using such a subclass, while having
fixed (in code) configuration sibling classes. Or add such a
fixed-config-singleton class method to the base class to add that to the
whole hierarchy. I don't have the need. But I could.
Any function that already uses the $redis = MyRedis::inst(); pattern
can then quickly be changed to $redis = MyRedis::inst('volatile'); and
continue to work fine.
Or, in my case, s/MyRedis::/MyRedisReg::inst('volatile')-/g. Piece of cake.
Extend was perhaps not the word I should have chosen; I was thinking of
extending the use of the class beyond the current coding pattern. By
implementing one specific use case as a magic method, you make it harder to
do anything other than that one use case; or, you end up creating a whole
set of magic methods and features, which are then no easier than just
implementing the whole thing by hand.
Nothing of that. By base class is fully flexible! It even throws an
exception when a coder tries to singleton-like use it itself - it's just
the concrete subclasses that can be used that way.
I consciously chose this nonextensibility. Of course this is (for the given
fanout of different redis servers) nothing that a public facing library
would use! I'm watching over a four man coder show writing a single web
site's code not indented for public consumption. And there, such things not
only make sense, they happen all over the place.
Anyway, I'm pretty sure that singleton subclasses are useful in various
other one-of-a-few-specializations cases for completely different uses, and
there the patterrn might just make sense, too.
I certainly understand your criticism, from a point of view of more
publically consumable codebases / libraries. Thanks for your feedback.
best regards
Patrick
Re: [PHP-DEV] A modest proposal: __contructStatic
Am 16.02.2015 02:40 schrieb Kris Craig kris.cr...@gmail.com: I've never really done any work with Redis before so I'm also having trouble understanding the use case for this given that everybody's talking about this solely in the context of Redis. Ahem, it's not everbody, just me :) And the issue comes up in the context of redis because persistent connection without keeping around yet another global $myglobalredis, and ease of use to me in that concrete case. Yeah. I had a use case. That's all. I'll rest my case soon if there are no others. best regards Patrick
Re: [PHP-DEV] A modest proposal: __contructStatic
Am 16.02.2015 00:05 schrieb Rowan Collins rowan.coll...@gmail.com: A quick thought - if you want to stick with the magic static call pattern, you can implement this much more simply by doing something similar to Laravel's facades [1]: ... This basically implements in userspace what you propose to add to the language, with the only caveat being that you must separate the concerns of adding extra functionality, and wrapping it in a static facade, into two separate classes. That was one of my intermediate steps before reaching the current approach, but it irked me that when switching code from MyRedis static/singleton usage to real instance usage I suddenly have to write a different class. In my scheme, where I can use red_global::get() I can as well use $redis = new red_global(); $redis-get(); without thinking about how that companion class was named. Sure, no big issue. But a small one... By itself, surely not enough reason for a language change :) It's all a question of how many other small such uses there would be. best regards Patrick
