Consider this common scenario:
I use some OOP library, that is a "black box" and I like it that way.
As part of the integration, I need to extend one of the library's
classes:
class App_bar extends Library_foo{
function __construct(){
//do whatever I need to do here.
}
}
So I write this code, and it works, and I'm happy.
Years go by and new releases of the library come out, steadily
improving features and fixing bugs etc.
But at some point, the library needed a resource in a __construct()
somewhere in the ancestry of Library_foo, to perform something at the
time of construction.
So they re-factored or re-implemented the black box library to have a
constructor in Library_foo class, or any of its numerous ancestors.
Suddenly my code doesn't work. Maybe the resource is only needed under
certain conditions. So their __construct doesn't get called, but
nothing bad happens until the circumstances where they need the
__construct called.
So now I don't just have a bug, I have an intermittent bug.
And as much as we all like to think our testing covers 100% of the
functionality...
I'm tearing my hair out trying to find where things went wrong in code
that worked for years.
Right now, to avoid this situation, you have to do:
if (method_exists(get_parent_class(), '__construct'))
parent::__construct();
If you don't check for the method existing, you get:
Fatal error: Cannot call constructor ...
I don't want to have an intimate knowledge of the internals of the
library; just the documented API. That's one of the major reasons for
using a library.
I would think that as PHP climbs up the chain of "extends" et al, if
it never found a parent method, if wouldn't complain about it, much
less E_FATAL...
I mean, so you didn't find a method that's not there to find. And I
should call parent::__construct just in case there is one there,
because I don't know, or care, really, how the black box is
implemented. So long as the library does what's on the tin (in the
docs).
Hi!
Right now, to avoid this situation, you have to do:
if (method_exists(get_parent_class(), '__construct'))
parent::__construct();If you don't check for the method existing, you get:
Fatal error: Cannot call constructor ...
This makes a lot of sense. I think we also discussed this idea some time
ago, but it didn't go anywhere that time... Right now I add empty ctor
in almost all base classes just because of this thing. I think having
implicit empty ctor would make a lot of things easier. Same would
probably do for dtor. How about creating an RFC about it? :)
--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227
On Thu, May 23, 2013 at 11:32 PM, Stas Malyshev smalyshev@sugarcrm.comwrote:
Hi!
Right now, to avoid this situation, you have to do:
if (method_exists(get_parent_class(), '__construct'))
parent::__construct();If you don't check for the method existing, you get:
Fatal error: Cannot call constructor ...This makes a lot of sense. I think we also discussed this idea some time
ago, but it didn't go anywhere that time...
last time it was discussed on the list:
internals@lists.php.net/msg50504.html" rel="nofollow" target="_blank">http://www.mail-archive.com/internals@lists.php.net/msg50504.html
to summarize it:
Etienne was arguing that for the concrete example (Spl classes) it would be
better to make sure that the object is properly initialized so having a
constructor in the spl classes.
Anthony was the only one who was against the idea of removing the error
when you call a parent constructor which doesn't (explicitly) have one.
Otherwise everybody else seemed to agree with the above change and only the
implementation was discussed:
- we should remove the error completelly
- we should make the error less serious (please don't do this)
- we should always create a common ancestor for every class by default
which have an empty constructor/destructor - we should always create an empty constructor/desctructor for every class
without it.
So maybe we could discuss this and based on the feedback create an rfc
(with or without a vote for the alternative implementations).
Ferenc Kovács
@Tyr43l - http://tyrael.hu
On Thu, May 23, 2013 at 11:32 PM, Stas Malyshev <smalyshev@sugarcrm.com
wrote:
Hi!
Right now, to avoid this situation, you have to do:
if (method_exists(get_parent_class(), '__construct'))
parent::__construct();If you don't check for the method existing, you get:
Fatal error: Cannot call constructor ...This makes a lot of sense. I think we also discussed this idea some time
ago, but it didn't go anywhere that time...last time it was discussed on the list:
internals@lists.php.net/msg50504.html" rel="nofollow" target="_blank">http://www.mail-archive.com/internals@lists.php.net/msg50504.html
to summarize it:
Etienne was arguing that for the concrete example (Spl classes) it would be
better to make sure that the object is properly initialized so having a
constructor in the spl classes.
Anthony was the only one who was against the idea of removing the error
when you call a parent constructor which doesn't (explicitly) have one.
Otherwise everybody else seemed to agree with the above change and only the
implementation was discussed:
- we should remove the error completelly
- we should make the error less serious (please don't do this)
- we should always create a common ancestor for every class by default
which have an empty constructor/destructor- we should always create an empty constructor/desctructor for every class
without it.So maybe we could discuss this and based on the feedback create an rfc
(with or without a vote for the alternative implementations).
Hi.
I'm not an expert here, so just thinking out loud ...
If a theoretical \PHP\baseclass can have empty __construct()/__destruct(),
what about the other magic methods?
OK, I suppose cascading some of the magic methods to a parent and having a
null parent at the very very bottom of the heap sounds useful. But I'm not
totally sure.
Is there much/any need for a true base class that ALL classes will extend
from, including those in extensions.
Richard.
--
Richard Quadling
Twitter : @RQuadling
EE : http://e-e.com/M_248814.html
Zend : http://bit.ly/9O8vFY
Hi.
I'm not an expert here, so just thinking out loud ...
If a theoretical \PHP\baseclass can have empty __construct()/__destruct(),
what about the other magic methods?OK, I suppose cascading some of the magic methods to a parent and having a
null parent at the very very bottom of the heap sounds useful. But I'm not
totally sure.Is there much/any need for a true base class that ALL classes will extend
from, including those in extensions.
it was discussed in the linked thread, personally I agree that those are
different both in intention and implementation, so shouldn't affected by
this change.
ps: for example having an empty __sleep() or __wakeup implementation would
be entirely differrent that one would expect.
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
Sure the default implementation would have to be identical to the behavior
of not defining one.
I believe the best way to solve these issues is by having an implicit base
class. To some extent, that means BC breaks though.
Hi.
I'm not an expert here, so just thinking out loud ...
If a theoretical \PHP\baseclass can have empty
__construct()/__destruct(), what about the other magic methods?OK, I suppose cascading some of the magic methods to a parent and having
a null parent at the very very bottom of the heap sounds useful. But I'm
not totally sure.Is there much/any need for a true base class that ALL classes will extend
from, including those in extensions.it was discussed in the linked thread, personally I agree that those are
different both in intention and implementation, so shouldn't affected by
this change.ps: for example having an empty __sleep() or __wakeup implementation would
be entirely differrent that one would expect.--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
--
Etienne Kneuss
http://www.colder.ch
Sure the default implementation would have to be identical to the behavior
of not defining one.
agree
I believe the best way to solve these issues is by having an implicit base
class.
that would also solve the "I want to typehint objects" problem.
To some extent, that means BC breaks though.
by BC break you mean the name of the implicit base class?
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
Sure the default implementation would have to be identical to the
behavior of not defining one.agree
I believe the best way to solve these issues is by having an implicit
base class.that would also solve the "I want to typehint objects" problem.
To some extent, that means BC breaks though.
by BC break you mean the name of the implicit base class?
No, I was more referring to the fact that people might be relying on the
hierarchy to be bounded by their own classes, and checking for their roots
using i.e. class_parents/reflection.
If we introduce a class and force it as parent of every userland root
classes, you can always find existing code to break in non-trivial ways :)
It seems acceptable to me though, given the progress we would make with
this change.
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
--
Etienne Kneuss
Encouraging call super is still not the right way to go about this. As
I said before, even with the changes proposed, there's no way to
contractually enforce the call super pattern in this language.
That's why it is and will remain an antipattern. So all you're doing
is allowing people to more easily use the antipattern, instead of (a)
baking auto-super (contractual call super) into the language or (b)
pushing people to use a preferred pattern (Template Method or other).
Template Method in userland is not difficult (lightly tested):
trait templateInitializer {
private $inited;
final public function __construct( $arg1, $arg2 ) {
print "__construct in super calling inits...<BR>";
$this->init( $arg1, $arg2 );
$this->inited or self::init( $arg1, $arg2 );
}
protected function init ( $arg1, $arg2 ) { print "init in super...<BR>"; $this->inited = true; }
}
class superClass3 {
use templateInitializer;
}
class subClass3a extends superClass3 {
protected function init ( $arg1, $arg2 ) { print "init in sub...<BR>"; }
}
class subClass3b extends superClass3 {}
class subSubClass3a1 extends subClass3a {
protected function init ( $arg1, $arg2 ) { print "init in sub-sub...<BR>"; }
}
new superClass3 ( null, null ); // runs root init
new subClass3a ( null, null ); // runs child init, then root
new subClass3b ( null, null ); // runs root init
new subSubClass3a1 (null, null); // runs grandchild, then root
You can extend this model to preInit() and postInit() methods, giving
even more flexibility. And in none of these cases does the subclass
need to "remember" to do anything.
This thread arose in response to what is undeniably a bad API
authoring practice (first allowing full ctor override and later
breaking BC without fanfare). Why are we letting the authors of
Richard's library off the hook and putting it on ourselves to develop
hacks? They obviously screwed up. What next? Do we let API consumers
override a final that they don't like? Just use the right pattern in
the first place. IMO.
-- S.
This thread arose in response to what is undeniably a bad API
authoring practice (first allowing full ctor override and later
breaking BC without fanfare) Why are we letting the authors of
Richard's library off the hook and putting it on ourselves to develop
hacks? They obviously screwed up. What next? Do we let API consumers
override afinalthat they don't like? Just use the right pattern in
the first place. IMO.
I disagree.
It can be perfectly ok to allow the lib to be extended and the constructor
extended/replaced with a compatible one.
The problem is, that currently there is a scenario(lib didn't had a
constructor but now it does) which is a PITA.
Either the lib author needs to add an empty constructor into every class
just to make sure that it will be there to provide a painless upgrade when
the need arises for a constructor otherwise the lib consumer has to write
boilerplate to make sure that the parent constructor will be only called
whet ot is there.
Your example also shows an overengineered solution for a simple problem.
I think that this is QoL improvement which can be implemented to have a
reasonable gain without any cons.
It can be perfectly ok to allow the lib to be extended and the constructor
extended/replaced with a compatible one.
Well sure, it's great to override constructors completely. If the lib
authors didn't want you to do that, they should've made it final
(which is what I said they should do now). You have every reason to
expect this will work and keep working. Without call super.
But "extending" a constructor in some made-up way where you override
and just guess you need to call super if one exists -- after your
additional code? before? why one over the other? -- without any formal
documentation at all about the consequences? I think that's a stink
bomb and I'm sticking with that view. It's like you're treating the
superctor as simultaneously opaque and knowable.
Say it happens to work at first b/c you don't touch any resources that
the superctor needs, or you assign them and the superctor runs
afterward and resets them to a workable state (unbeknownst to you).
For the sake of argument, let's call this compatibility instead of
luck. And how do you keep it compatible? You don't. They could change
the superctor signature (oops! you're out of luck because you don't
pass them what they need). Internally, they could change what
resources superctor expects you to leave alone, what it doesn't, and
in what order. It's simple to break this kind of thing, where a
third-party doesn't want you to do something but they forgot to stop
you.
Either the lib author needs to add an empty constructor into every class
just to make sure that it will be there to provide a painless upgrade when
the need arises for a constructor
There's no guarantee at all that it will be painless, because the
signatures could mismatch, not to mention every single other
dependency being subject to your guesswork. If the authors change
their approach, it should be to the Template Method/subinit pattern,
not making an "official" Call Super reqt that'll still be doomed to
failure.
otherwise the lib consumer has to write boilerplate to make sure
that the parent constructor will be only called whet ot is there.
Which is still just a guess, and basically doomed. Why would you think
you can run an opaque parent:__construct() at any time? I would never
just trust that. And I'm hardly risk-averse. :)
Look, my gripe isn't with silencing/catching the call to unknown
methods in lieu of method_exists(). If that gets easier to do in the
language, fine. But I think this use case stinks as a justification.
Still, I'll lay off after this e-mail because I will have said my
piece. :P
Your example also shows an overengineered solution for a simple problem.
But it's 4 lines long! The rest is comments and examples of usage. And
it's a real solution to the problem, so it could only be
overengineered vs. something else that is functionally equivalent.
The Template Method pattern ensures that the child classes do not need
to "remember" to invoke a particular method within their initializer.
Simply by having an init() method in your derived class, you ensure
that upper-level dependencies will be taken care of, and you don't
have to have an init() method at all if you don't want to, or you can
have an empty one with no side effects. The initialization hierarchy
is baked into the object model at the outermost possible layer (since
you can't go one more level out into language constructs). This
doesn't mean you don't need documentation of what you should/shouldn't
do when you init, but it is way closer to being manageable over time.
The Call Super pattern requires that the user remember to call within
their ctor code, and even worse, the situation under discussion is
truly the worst possible argument for the pattern, since you don't
even have any word from the library author that call super is
recommended or required... so not only can you not enforce the
contract, you're making up the notion of the contract off the top of
your head.
Cheers regardless --
-- S.
I use some OOP library, that is a "black box" and I like it that way.
Hmm, there's "well-documented, change-controlled, trustable API that
you shouldn't try to work around" and then there's "black box." I'm
not sure the latter ever sounds like a good thing... I've always used
it as a bad word.
Suddenly my code doesn't work. Maybe the resource is only needed under
certain conditions. So their __construct doesn't get called, but
nothing bad happens until the circumstances where they need the
__construct called.
If their __construct suddenly becomes mandatory, they are deprecating
something they (I presume explicitly) supported in the past: ctor
overrides without call super.
So either they put this in giant blinking text in their UPDATING file
and you missed it, or they broke backward compatibility and didn't
know it or didn't care.
If I were faced with their situation, I'd make the ctor final to make
damn sure you realized something was wrong. Then I would say
overriding __construct is no longer supported because continuing to
support it means mandating call super, which is an impossible contract
for the language (this anguage) to enforce. I would abstract the whole
thing out to an initChild() type pattern and so on. If you have to
change your code, them's the breaks, as long as they give a clear
path.
I don't think the solution is to work around such mistakes on the part
of a lib developer by silencing (?) calls to a nonexistent method in
addition to mandating call super -- IMO, that's even smellier than
call super on its own. Why not add auto-super contracts to PHP instead
(which would include skipping nonexistent super methods)?
Making the undefined method error catchable instead of fatal I agree
with, though. More like the more catchable world of Reflection.
-- Sandy
First, thanks for all the comments!
Responding in-line to myself to address everything so far in this thread:
Consider this common scenario:
I use some OOP library, that is a "black box" and I like it that way.
This was a made-up scenario.
Well, somewhat...
In my experience, this sort of thing usually happens in the corporate
environment with Jr. Programmer, rather than in publicly-released
libraries.
Educating Jr. Programmer regarding documentation needs to happen and
will happen, but it will probably be out-of-band from using their
code.
if (method_exists(get_parent_class(), '__construct'))
parent::__construct();
99% of the time, I need to call their constructor before mine to
initialize stuff.
I'm very pragmatic.
If PHP had default __construct, and in the absence of [decent] docs
for the parent, I'm going to call the parent constructor first and
pray.
I thought all PHP user-land class were rooted in stdClass. The class
functions that display hierarchy obscure that, but there it's there
under the hood.
If so, adding the magic anti-pattern methods to stdClass would be
easy, right?
Extensions that return PHP objects should get the "free ride" from
stdClass, I would expect.
The sample init() pattern or whatever from Etienne made my eyes
un-focus in the first couple lines.
I'm not saying it's not the better pattern, or even that what I
propose isn't an anti-pattern.
Just it was way too complicated for what I need 99% of the time.
I'm sure if I needed that particular pattern's properties it would
make a whole lot more sense.
My thoughts on which magic methods [don't] fit this treatment:
DO FIT
__construct: 99% I let them init their stuff, and I init mine
__destruct: I tear down my stuff, and call the parent
__toString: I would imagine children "tacking on"
(actually, I think parent::__toString being safe might be baked in
already, as all classes have a toString, right?...)
__call*: Parent first, do my stuff, usually return parent result
__invoke? Never used it, don't like it, basing on description
DO NOT FIT
__sleep
__wakeup
__get
__set
__isset
__set_state Already effectively calls all descendants if the first
note is correct...
__clone Already has an :after method __done() to deal with this
I'm not particularly strongly for or against any of them except
__construct __destruct
Well, __set_state and __clone pretty much have to be the NOT FIT pile,
as far as I can see.
Again, I'm VERY pragmatic. The base object having __construct and
__destruct so I can call them in FIFO/LIFO order covers 99% of the use
cases.
If I was writing an RFC today, and I'm not until there is a little
more discussion on-list, I would explicitly include only ctor/dtor in
it as a trial, and leave the rest for "later". Or "never" if that's
the way it pans out.
Richard et al,
Consider this common scenario:
I use some OOP library, that is a "black box" and I like it that way.
As part of the integration, I need to extend one of the library's
classes:
The problem with this entire thread (I've been thinking about this for a
while) is that it's setup from the very beginning with a failed assertion.
You want to treat a library as a black box. Great! However, you also want
to extend one of the library's classes. That's not ok. Because you cannot
ever treat inheritance as a black box. It goes against just about every
principle out there. And it's provably not possible to do.
Treating it like a black box will cause LSP violations (which PHP is so
keen on enforcing). Why? Because if you treat the parent as a black-box,
you can't guarantee that the pre-conditions, post-conditions
and invariants are not enforced (which LSP requires). So without looking at
the parent, you can't have a child that adheres to LSP.
But let's get less "rule" and more practical. Imagine the parent has no
constructor. So you, in future hindsight, decide to do this:
if (method_exists(get_parent_class(), '__construct')) {
call_user_func_array(array(get_parent_class(), '__construct'),
func_get_args());
}
Now, what happens if the parent constructor now takes a parameter by
reference? The reference will be lost. The conceptual implementation is now
gone. Not to mention that the parameters may (and will likely) be different
from what you were expecting.
So basically, we can't ever develop completely in a black box. So if the
parent has a constructor, we need to call it (or not). But the point is,
it's never a conditional in the code. It requires you to look at, and make
a decision.
So that brings us to the case when you override a class that doesn't have a
constructor, which then gets one later on down the road. This is really
just the same as before and boils down to: 1. Test your code, as a test
should catch that. 2. Don't blindly update and expect code to work. 3. Even
if you did have a conditional, you're likely to get the arguments wrong (or
worse)...
So realistically, while I can see the appeal to having the ability to
always call parent::__construct(), I think it's actually a red-herring to
the actual problem. And it's not really necessary in the first place. In
fact, using it is likely to be a source of more bugs, as the object still
won't be initialized properly (but you think it is)...
My $0.02 at least,
Anthony
2013/5/30 Anthony Ferrara ircmaxell@gmail.com
Richard et al,
Consider this common scenario:
I use some OOP library, that is a "black box" and I like it that way.
As part of the integration, I need to extend one of the library's
classes:The problem with this entire thread (I've been thinking about this for a
while) is that it's setup from the very beginning with a failed assertion.
You want to treat a library as a black box. Great! However, you also want
to extend one of the library's classes. That's not ok. Because you cannot
ever treat inheritance as a black box. It goes against just about every
principle out there. And it's provably not possible to do.
I cannot agree with you here. This is exactly why the "protected" keyword
exists: to provide a black-box interface for class usage through
inheritance. Yes, this is not automatic and, yes it needs designing, but it
is certainly not against the principles. If it were, we should remove the
"protected" keyword as well.
Treating it like a black box will cause LSP violations (which PHP is so
keen on enforcing). Why? Because if you treat the parent as a black-box,
you can't guarantee that the pre-conditions, post-conditions
and invariants are not enforced (which LSP requires). So without looking at
the parent, you can't have a child that adheres to LSP.But let's get less "rule" and more practical. Imagine the parent has no
constructor. So you, in future hindsight, decide to do this:if (method_exists(get_parent_class(), '__construct')) {
call_user_func_array(array(get_parent_class(), '__construct'),
func_get_args());
}Now, what happens if the parent constructor now takes a parameter by
reference? The reference will be lost. The conceptual implementation is now
gone. Not to mention that the parameters may (and will likely) be different
from what you were expecting.So basically, we can't ever develop completely in a black box. So if the
parent has a constructor, we need to call it (or not). But the point is,
it's never a conditional in the code. It requires you to look at, and make
a decision.So that brings us to the case when you override a class that doesn't have a
constructor, which then gets one later on down the road. This is really
just the same as before and boils down to: 1. Test your code, as a test
should catch that. 2. Don't blindly update and expect code to work. 3. Even
if you did have a conditional, you're likely to get the arguments wrong (or
worse)...So realistically, while I can see the appeal to having the ability to
always call parent::__construct(), I think it's actually a red-herring to
the actual problem. And it's not really necessary in the first place. In
fact, using it is likely to be a source of more bugs, as the object still
won't be initialized properly (but you think it is)...My $0.02 at least,
Anthony
Lazare INEPOLOGLOU
Ingénieur Logiciel
Lazare,
I cannot agree with you here. This is exactly why the "protected" keyword
exists: to provide a black-box interface for class usage through
inheritance. Yes, this is not automatic and, yes it needs designing, but it
is certainly not against the principles. If it were, we should remove the
"protected" keyword as well.
The protected keyword is there to provide encapsulation. Meaning to allow
the responsibilities to be encapsulated within the object tree, rather than
being publicly exposed. It is not there to be a "black box", as you still
need to bind to a non-public interface to use it. It doesn't have anything
to do with providing a "black box interface". It's about delegating
encapsulation, but once you extend into a class tree, any notion of being a
black box by very definition has to go away (especially since the base
class can override or write to any of your protected properties as well)...
Anthony