thekid@friebes:~/devel/php/tests > cat inheritance.php
<?php
class Foo {
function __construct($foo) {
}
}
class Bar extends Foo {
function __construct($foo, $bar) {
// Add = NULL
after $bar to make it work
}
}
?>
thekid@friebes:~/devel/php/tests > php-dev inheritance.php
Fatal error: Declaration of Bar::__construct() must be compatible with
that of Foo::__construct() in
/usr/home/thekid/devel/php/tests/inheritance.php on line 10
Is this really necessary?
- Timm
Hello Timm,
i had the same expirience today too. And also for me it makes not much
sense. The constructor shouldn't check inheritance rules. And as a
consequence maybe interfaces shouldn't allow constructors.
marcus
Wednesday, February 25, 2004, 11:36:57 PM, you wrote:
thekid@friebes:~/devel/php/tests > cat inheritance.php
<?php
class Foo {
function __construct($foo) {
}
}
class Bar extends Foo {
function __construct($foo, $bar) {
// Add =NULL
after $bar to make it work
}
}
?>>
thekid@friebes:~/devel/php/tests > php-dev inheritance.php
Fatal error: Declaration of Bar::__construct() must be compatible with
that of Foo::__construct() in
/usr/home/thekid/devel/php/tests/inheritance.php on line 10
Is this really necessary?
- Timm
--
Best regards,
Marcus mailto:helly@php.net
Hello Timm,
i had the same expirience today too. And also for me it makes not much
sense. The constructor shouldn't check inheritance rules.
Neither should other methods follow this. What if I want to add a
non-default parameter to an overriden method?
<?php
class Foo {
function connect($server) {
}
}
class Bar extends Foo {
function connect($server, $port) {
}
}
?>
I see where the problem comes from:
zend_do_perform_implementation_check() is called from
do_inherit_method_check() (both in zend_compile.c) which in turn is
called for inheritance and for interfaces. The behaviour is fully
desirable when implementing interfaces but not for regular inheritance.
- Timm
Hello Timm,
well for normal methods we must do that. The derived class must support the
same signature that the base class supports. In you example that would only
work if the derived method would have a default parameter for the additional
parameter:
<?php
class Foo {
function connect($server) {
}
}
class Bar extends Foo {
function connect($server, $port = NULL) {
}
}
?>
regards
marcus
Wednesday, February 25, 2004, 11:52:20 PM, you wrote:
Hello Timm,
i had the same expirience today too. And also for me it makes not much
sense. The constructor shouldn't check inheritance rules.
Neither should other methods follow this. What if I want to add a
non-default parameter to an overriden method?
<?php
class Foo {
function connect($server) {
}
}
class Bar extends Foo {
function connect($server, $port) {
}
}
?>>
I see where the problem comes from:
zend_do_perform_implementation_check() is called from
do_inherit_method_check() (both in zend_compile.c) which in turn is
called for inheritance and for interfaces. The behaviour is fully
desirable when implementing interfaces but not for regular inheritance.
- Timm
--
Best regards,
Marcus mailto:helly@php.net
well for normal methods we must do that. The derived class must support the
same signature that the base class supports. In you example that would only
work if the derived method would have a default parameter for the additional
parameter:<?php
class Foo {
function connect($server) {
}
}class Bar extends Foo {
function connect($server, $port = NULL) {
}
}
?>
When was this change made? I have a CVS check out from yesterday and
this is not enforced. This will breaks lots of PHP 4 code.
-adam
--
adam@trachtenberg.com
author of o'reilly's php cookbook
avoid the holiday rush, buy your copy today!
Hello Timm,
well for normal methods we must do that. The derived class must support the
same signature that the base class supports. In you example that would only
work if the derived method would have a default parameter for the additional
parameter:
I think this is only annoying users for nothing... I really don't see a
good reason on why to enforce this.
Derick
Hello Timm,
i had the same expirience today too. And also for me it makes not much
sense. The constructor shouldn't check inheritance rules.Neither should other methods follow this. What if I want to add a
non-default parameter to an overriden method?
Some test cases:
Should work #1, Bar::connect() adds an argument
<?php
interface Connector {
function connect($server);
}
class Foo implements Connector {
function connect($server) { }
}
class Bar extends Foo {
function connect($server, $port) { }
}
?>
Should work #2, Bar::connect() might contain something such as
parent::connect('foo.example.com');
<?php
interface Connector {
function connect($server);
}
class Foo implements Connector {
function connect($server) { }
}
class Bar extends Foo {
function connect() { }
}
?>
Should work #3, Bar::connect() might contain something such as
parent::connect($dsn->getHost());
<?php
class DSN { }
interface Connector {
function connect($server);
}
class Foo implements Connector {
function connect($server) { }
}
class Bar extends Foo {
function connect(DSN $dsn) { }
}
?>
Should work #4, Foo::connect() adds a default argument
<?php
interface Connector {
function connect($server);
}
class Foo implements Connector {
function connect($server, $port= 42) { }
}
class Bar extends Foo {
function connect($server, $port= 23) { }
}
?>
Should work #5, both interface and implementer have a default value
for the argument "server"
<?php
interface Connector {
function connect($server= 'localhost');
}
class Foo implements Connector {
function connect($server= 'localhost') { }
}
class Bar extends Foo {
function connect($server= 'localhost', $port= 23) { }
}
?>
Should work #6, implementation adds a default value
<?php
interface Connector {
function connect();
}
class Foo implements Connector {
function connect($server= 'localhost') { }
}
class Bar extends Foo {
function connect($server= 'localhost', $port= 23) { }
}
?>
Should NOT work #1, Foo doesn't fully implement connect()
<?php
interface Connector {
function connect($server);
}
class Foo implements Connector {
function connect() { }
}
class Bar extends Foo {
function connect($server, $port) { }
}
?>
Should NOT work #2, class / primitive clash on argument
<?php
class DSN { }
interface Connector {
function connect(DSN $dsn);
}
class Foo implements Connector {
function connect($dsn) { }
}
class Bar extends Foo {
function connect($dsn) { }
}
?>
Should NOT work #3, Foo implements Connector::connect() incorrectly
<?php
interface Connector {
function connect($server);
}
class Foo implements Connector {
function connect($server, $port) { }
}
class Bar extends Foo {
function connect($server, $port= 23) { }
}
?>
Should NOT work #4, Foo implements Connector::connect() incorrectly
<?php
interface Connector {
function connect();
}
class Foo implements Connector {
function connect($server) { }
}
class Bar extends Foo {
function connect($server, $port) { }
}
?>
I would simply refrain from calling
zend_do_perform_implementation_check() in inheritance.
- Timm
Hello Timm,
Thursday, February 26, 2004, 12:34:31 AM, you wrote:
Hello Timm,
i had the same expirience today too. And also for me it makes not much
sense. The constructor shouldn't check inheritance rules.Neither should other methods follow this. What if I want to add a
non-default parameter to an overriden method?
Some test cases:
Should work #1, Bar::connect() adds an argument
No the sugnature is incompatible. An instance of Foo cannot be called
with Bar or Connector's connect() Signature. Hence Bar is not a Foo
or Connector.<?php
interface Connector {
function connect($server);
}
class Foo implements Connector {
function connect($server) { }
}
class Bar extends Foo {
function connect($server, $port) { }
}
?>>
Should work #2, Bar::connect() might contain something such as
Same asas above
parent::connect('foo.example.com');<?php
interface Connector {
function connect($server);
}
class Foo implements Connector {
function connect($server) { }
}
class Bar extends Foo {
function connect() { }
}
?>>
Should work #3, Bar::connect() might contain something such as
Same as above. Foo::connect() can be called with any value
Bar::connect() not.
parent::connect($dsn->getHost());<?php
class DSN { }
interface Connector {
function connect($server);
}
class Foo implements Connector {
function connect($server) { }
}
class Bar extends Foo {
function connect(DSN $dsn) { }
}
?>>
Should work #4, Foo::connect() adds a default argument
Should work, since the calling convention from Connector is still
possible in Foo and Bar. Hence Foo::connect() IS-A Connector::connect()
and Bar::connect() IS-A Connector::connect().<?php
interface Connector {
function connect($server);
}
class Foo implements Connector {
function connect($server, $port= 42) { }
}
class Bar extends Foo {
function connect($server, $port= 23) { }
}
?>>
Should work #5, both interface and implementer have a default value
for the argument "server"
Should work. Connector's calling convention is till valid in Bar.
Hence Foo::connect() IS-A Connector::connect() and Bar::connect()
IS-A Connector::connect().<?php
interface Connector {
function connect($server= 'localhost');
}
class Foo implements Connector {
function connect($server= 'localhost') { }
}
class Bar extends Foo {
function connect($server= 'localhost', $port= 23) { }
}
?>>
Should work #6, implementation adds a default value
Same as #5 only more complicated.<?php
interface Connector {
function connect();
}
class Foo implements Connector {
function connect($server= 'localhost') { }
}
class Bar extends Foo {
function connect($server= 'localhost', $port= 23) { }
}
?>>
Should NOT work #1, Foo doesn't fully implement connect()
Right shouldn't work.<?php
interface Connector {
function connect($server);
}
class Foo implements Connector {
function connect() { }
}
class Bar extends Foo {
function connect($server, $port) { }
}
?>>
Should NOT work #2, class / primitive clash on argument
Wrong. Should work. Foo::connect() IS-A Connector::connect() and
Bar::connect() IS-A Connector::connect(). It is no problem that a
derived clsses method accepts a greater value range or set of types.<?php
class DSN { }
interface Connector {
function connect(DSN $dsn);
}
class Foo implements Connector {
function connect($dsn) { }
}
class Bar extends Foo {
function connect($dsn) { }
}
?>>
Should NOT work #3, Foo implements Connector::connect() incorrectly
Should work. Still Foo::connect() IS-A Connector::connect()
and Bar::connect() IS-A Connector::connect().<?php
interface Connector {
function connect($server);
}
class Foo implements Connector {
function connect($server, $port) { }
}
class Bar extends Foo {
function connect($server, $port= 23) { }
}
?>>
Should NOT work #4, Foo implements Connector::connect() incorrectly
Right, shouldn't work. Inheritance doesn't apply since the signatures
are incompatible.<?php
interface Connector {
function connect();
}
class Foo implements Connector {
function connect($server) { }
}
class Bar extends Foo {
function connect($server, $port) { }
}
?>>
I would simply refrain from calling
zend_do_perform_implementation_check() in inheritance.
- Timm
--
Best regards,
Marcus mailto:helly@php.net
Hello Timm,
[...]Should work #1, Bar::connect() adds an argument
No the sugnature is incompatible. An instance of Foo cannot be called
with Bar or Connector's connect() Signature. Hence Bar is not a Foo
or Connector.
Hrm, that's quite a (huge) BC break then. I know that adding a parameter
kind-of violates the contract between Bar and Connector, but - for an
instance - omit the interface idea and think about this:
class Error {
function __construct($message) { }
}
class SQLError extends Error {
function __construct($message, $sql) { }
}
This gives us an error. Or, not to restrict this to constructors:
class Printer {
function print() { }
}
class MultipleFormatCapablePrinter extends Printer {
function print($format) { }
}
This works just fine in PHP4, where, if I call SQLError's constructor
with one argument only, I'll simply get an E_WARNING.
You're changing this to a E_COMPILE_ERROR. Shouldn't some alarm bells
start ringing here?
With this new requirement, I'd have to make all additional parameters
optional in subclasses. This introduces more kludges:
class MultipleFormatCapablePrinter extends Printer {
function print($format= NULL) {
if (NULL === $format) {
throw new IllegalArgumentException('Format may not be NULL');
}
}
}
Now users can call this method (they could find out, e.g., via
Reflection_Method that format is an optional parameter - which means:
it can be ommitted, and if it is, it'll just be NULL
- but it actually
can't, it's only declared with a default value because of a language
requirement) and won't even get an error if I don't catch it myself.
This doesn't come up in Java, as they have method overloading, so
MultipleFormatCapablePrinter::print() is actually a different method as
Printer::print() and you get away with it. And yes, PHP isn't Java.
[...]
Should NOT work #2, class / primitive clash on argument
Wrong. Should work. Foo::connect() IS-A Connector::connect() and
Bar::connect() IS-A Connector::connect(). It is no problem that a
derived clsses method accepts a greater value range or set of types.
Well, this one:
<?php
class DSN { }interface Connector {
function connect(DSN $dsn);
}class Foo implements Connector {
function connect($dsn) { }
}class Bar extends Foo {
function connect($dsn) { }
}
?>
doesn't work right now, see zend_compile.c, lines 1737 - 1744. There's
even an inline comment (!) about it there:)
Even more weirdness:
thekid@friebes:~/devel/php/tests > cat inheritance.php
<?php
interface Connector {
function connect();
}
class Foo implements Connector {
function connect($server) { }
}
?>
thekid@friebes:~/devel/php/tests > php-dev inheritance.php
thekid@friebes:~/devel/php/tests > cat inheritance-b0rked.php
<?php
interface Connector {
function connect($server);
}
class Foo implements Connector {
function connect($server, $port) { }
}
?>
thekid@friebes:~/devel/php/tests > php-dev inheritance-b0rked.php
Fatal error: Declaration of Foo::connect() must be compatible with that
of Connector::connect() in
/usr/home/thekid/devel/php/tests/inheritance-b0rked.php on line 6
Huh? So having zero arguments in the interface and one in the
implementation is OK but having one in the interface and two in the
implementation is not?
- Timm
Hello Timm,
Hello Zeev, please have a look at this thread. Obviously you started
a huge BC break here. See my second comment on a possible way out.
Thursday, February 26, 2004, 2:13:48 AM, you wrote:
Hello Timm,
[...]Should work #1, Bar::connect() adds an argument
No the sugnature is incompatible. An instance of Foo cannot be called
with Bar or Connector's connect() Signature. Hence Bar is not a Foo
or Connector.
Hrm, that's quite a (huge) BC break then. I know that adding a parameter
kind-of violates the contract between Bar and Connector, but - for an
instance - omit the interface idea and think about this:
class Error {
function __construct($message) { }
}
class SQLError extends Error {
function __construct($message, $sql) { }
}
This gives us an error.
As i said i don't like this to be an error for constructors.
Or, not to restrict this to constructors:
class Printer {
function print() { }
}
class MultipleFormatCapablePrinter extends Printer {
function print($format) { }
}
This works just fine in PHP4, where, if I call SQLError's constructor
with one argument only, I'll simply get an E_WARNING.
You're changing this to a E_COMPILE_ERROR. Shouldn't some alarm bells
start ringing here?
Yes! Maybe it would be good to apply the correct rules with E_COMPILE_ERROR
in cases where interfaces come into play and E_STRICT
for compatibility mode
and non interfaces. Would that work for you?
With this new requirement, I'd have to make all additional parameters
optional in subclasses. This introduces more kludges:
class MultipleFormatCapablePrinter extends Printer {
function print($format= NULL) {
if (NULL === $format) {
throw new IllegalArgumentException('Format may not be NULL');
}
}
}
Now users can call this method (they could find out, e.g., via
Reflection_Method that format is an optional parameter - which means:
it can be ommitted, and if it is, it'll just beNULL
- but it actually
can't, it's only declared with a default value because of a language
requirement) and won't even get an error if I don't catch it myself.
This doesn't come up in Java, as they have method overloading, so
MultipleFormatCapablePrinter::print() is actually a different method as
Printer::print() and you get away with it. And yes, PHP isn't Java.
[...]
Should NOT work #2, class / primitive clash on argument
Wrong. Should work. Foo::connect() IS-A Connector::connect() and
Bar::connect() IS-A Connector::connect(). It is no problem that a
derived clsses method accepts a greater value range or set of types.
Well, this one:
<?php
class DSN { }interface Connector {
function connect(DSN $dsn);
}class Foo implements Connector {
function connect($dsn) { }
}class Bar extends Foo {
function connect($dsn) { }
}
?>
doesn't work right now, see zend_compile.c, lines 1737 - 1744. There's
even an inline comment (!) about it there:)
Even more weirdness:
thekid@friebes:~/devel/php/tests > cat inheritance.php
<?php
interface Connector {
function connect();
}
class Foo implements Connector {
function connect($server) { }
}
?>>
thekid@friebes:~/devel/php/tests > php-dev inheritance.php
thekid@friebes:~/devel/php/tests > cat inheritance-b0rked.php
<?php
interface Connector {
function connect($server);
}
class Foo implements Connector {
function connect($server, $port) { }
}
?>>
thekid@friebes:~/devel/php/tests > php-dev inheritance-b0rked.php
Fatal error: Declaration of Foo::connect() must be compatible with that
of Connector::connect() in
/usr/home/thekid/devel/php/tests/inheritance-b0rked.php on line 6
Huh? So having zero arguments in the interface and one in the
implementation is OK but having one in the interface and two in the
implementation is not?
- Timm
--
Best regards,
Marcus mailto:helly@php.net
Hello Timm,
[...]
Yes! Maybe it would be good to apply the correct rules withE_COMPILE_ERROR
in cases where interfaces come into play andE_STRICT
for compatibility mode
and non interfaces. Would that work for you?
Yupp, that's perfect.
- Timm
Hello Timm,
[...]Should work #1, Bar::connect() adds an argument
No the sugnature is incompatible. An instance of Foo cannot be called
with Bar or Connector's connect() Signature. Hence Bar is not a Foo
or Connector.Hrm, that's quite a (huge) BC break then.
Maybe we'd all be happier with an E_STRICT
warning for inheritance and
an E_COMPILE_ERROR
for interfaces implementation.
- Timm
Hello Timm,
[...]Should work #1, Bar::connect() adds an argument
No the sugnature is incompatible. An instance of Foo cannot be called
with Bar or Connector's connect() Signature. Hence Bar is not a Foo
or Connector.Hrm, that's quite a (huge) BC break then.
Right, I think we should not break BC in this instance.
Derick
Timm Friebe wrote:
Hello Timm,
i had the same expirience today too. And also for me it makes not much
sense. The constructor shouldn't check inheritance rules.Neither should other methods follow this. What if I want to add a
non-default parameter to an overriden method?
Yes, I think this is the same issue that I brought up earlier related to
interfaces. I brought it up then as an inconsistency -- i.e. that you
couldn't override methods in interfaces & hence using interfaces was
limiting the OO inheritance that PHP supported when not using
interfaces. In brief the issue was that if you extend a class that
implemented an interface the extending class (subclass) had to also
implement the parent class' interface and was therefore not allowed to
override methods (w/ incompatible signatures) -- and not allowed to
implement a different interface which itself might specify incompatible
signatures.
It seems now that PHP is no longer inconsistent, but it also seems that
it is impossible to override methods w/ incompatible signature. Is that
a correct assessment? This is a pretty big difference from PHP4, then.
Personally, I can live with it :) -- just want to make sure I
understand it correctly.
Thanks,
Hans
Marcus Boerger wrote:
Hello Timm,
i had the same expirience today too. And also for me it makes not much
sense. The constructor shouldn't check inheritance rules. And as a
consequence maybe interfaces shouldn't allow constructors.
Does inheritance include visibility rules? ;)
Andrey
Hello Andrey,
Thursday, February 26, 2004, 1:08:19 PM, you wrote:
Marcus Boerger wrote:
Hello Timm,
i had the same expirience today too. And also for me it makes not much
sense. The constructor shouldn't check inheritance rules. And as a
consequence maybe interfaces shouldn't allow constructors.Does inheritance include visibility rules? ;)
You can only increase wivibility (pretected-> public) in a strict IS-A
modell as PHP uses.
--
Best regards,
Marcus mailto:helly@php.net
At 23:36 25/02/2004 +0100, Timm Friebe wrote:
thekid@friebes:~/devel/php/tests > cat inheritance.php
<?php
class Foo {
function __construct($foo) {
}
}class Bar extends Foo {
function __construct($foo, $bar) {
// Add =NULL
after $bar to make it work
}
}
?>
thekid@friebes:~/devel/php/tests > php-dev inheritance.phpFatal error: Declaration of Bar::__construct() must be compatible with
that of Foo::__construct() in
/usr/home/thekid/devel/php/tests/inheritance.php on line 10Is this really necessary?
Guys,
You are breaking the isA relationship. We fixed this so that from now on,
people will not make such mistakes anymore (I think it's the right way to
go, so that we don't leave broken functionality around).
You can enable compatibility mode to make this work or specify a default
value (as you saw) for $bar so that you are keeping the interface of the
base class.
Andi
Timm Friebe wrote:
thekid@friebes:~/devel/php/tests > cat inheritance.php
<?php
class Foo {
function __construct($foo) {
}
}class Bar extends Foo {
function __construct($foo, $bar) {
// Add =NULL
after $bar to make it work
}
}
?>
thekid@friebes:~/devel/php/tests > php-dev inheritance.phpFatal error: Declaration of Bar::__construct() must be compatible with
that of Foo::__construct() in
/usr/home/thekid/devel/php/tests/inheritance.php on line 10Is this really necessary?
If that error still occurs if you explicitely call the parent's
__construct (from the derived __construct), that's a bug IMO.
It's totally understandable to get such an error message in the case the
parent constructor gets called implicitely, but for an explicit call
there should neither be a warning or an error .
Cheers,
Michael