Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:8137
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Date: Thu, 26 Feb 2004 02:38:36 +0100
Reply-To: Marcus Boerger <helly@php.net>
Message-ID: <671303254671.20040226023836@marcus-boerger.de>
To: Timm Friebe <thekid@thekid.de>, Zeev Suraski <zeev@zend.com>
Cc: internals@lists.php.net
In-Reply-To: <1077758027.685.109.camel@localhost>
References: <1077748616.685.16.camel@localhost>
 <1781292779406.20040225234401@marcus-boerger.de>
 <1077749539.685.27.camel@localhost> <1077752071.685.57.camel@localhost>
 <1721299626562.20040226013808@marcus-boerger.de>
 <1077758027.685.109.camel@localhost>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Declaration of Bar::__construct() must be compatible with that of Foo::__construct()
From: helly@php.net (Marcus Boerger)

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:

> On Thu, 2004-02-26 at 01:38, Marcus Boerger 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 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




-- 
Best regards,
 Marcus                            mailto:helly@php.net