Class constructor behaviour

9 years ago by Dan Ackroyd — view source

unread

Hi,

I've been going through some bug reports, and have found there are
several bad behaviours related to class constructors that ought to be
corrected, but could only be done at a major version. The bad
behaviours are:

Constructors returning null

Several classes in PHP return null when there is a problem in the
parameters passed to their constructor e.g.

<?php

$mf = new MessageFormatter('en_US', '{this was made intentionally incorrect}');
if ($mf === null) {
echo "Surprise!";
}

This is pretty horrible and should be fixed by making sure that
constructors either return an object or throw an exception.
Additionally the exception policy for core (that was previously
discussed here: http://marc.info/?t=119263748000001&r=1&w=2 ) should
be updated so that any constructor returning NULL is considered a bug,
no matter what the ini settings are. To be clear, procedural code
should behave as before, with users expected to check for errors.

This would be a BC break for people who are handling the constructor
returning null currently, as they would need to wrap that code with an
try/catch block.

The list of classes that show this behaviour is:

finfo
PDO
Collator
IntlDateFormatter
MessageFormatter
NumberFormatter
ResourceBundle
IntlRuleBasedBreakIterator

Constructors give warning, but are then in an unusable state

Several constructors check the parameters that they are given.....and
then just give a warning when they are not acceptable. e.g.

<?php
//Reflection function is not meant to accept an array
$reflection = new ReflectionFunction([]);
//Warning: ReflectionFunction::__construct() expects parameter 1 to be
string, array given in...

var_dump($reflection->getParameters());
//Fatal error: ReflectionFunctionAbstract::getParameters(): Internal
error: Failed to retrieve the reflection object in..

Again, this should be fixed by changing the constructors to throw an
exception if the input parameters are not acceptable.

Although technically this would be a BC break, I can't think of any
sane situation where people would be depending on the current
behaviour.

The list of classes that I believe have this issue is:

UConverter
SplFixedArray
ReflectionFunction
ReflectionParameter
ReflectionMethod
ReflectionProperty
ReflectionExtension
ReflectionZendExtension
Phar
PharData
PharFileInfo

Constructor gives error

Some constructors check the parameters they are given, and then emit
either a 'Catchable fatal error' error or other error e.g.

<?php
$foo = new IntlGregorianCalendar(new StdClass);
//Output: Catchable fatal error: Object of class stdClass could not be
converted to string in..
?>

Despite the word catchable, this is not actually an exception that is
catchable, it is an error. It is only if the user has called
set_error_handler, and in the error handler they throw an exception
then the code, that the error would be catchable.

The only other class that has a similar behaviour is PDORow which
gives a 'Fatal error' which can't be intercepted, but can only be
logged through a function registered through
register_shutdown_function().

The list of classes that have these issues, is possibly not complete.
The ones I've listed above was not done through an exhaustive search.

So, questions:

i) Can anyone see a big hurdle in fixing these behaviours, other than
it being a BC break for people who are currently relying on these
behaviours ?

ii) Are there any other bad behaviours that people are aware of that
ought to be fixed at a major version?

These changes would need an RFC to be approved I presume. However
doing the RFC would probably require making a patch that implements
most, if not all, of these changes. As that would be a significant
amount of work, I thought I'd get people's feedback before starting
that.

cheers
Dan

9 years ago by francois@tekwire.net — view source

unread

De : Dan Ackroyd [mailto:danack@basereality.com]

This is pretty horrible and should be fixed by making sure that
constructors either return an object or throw an exception.
Additionally the exception policy for core (that was previously
discussed here: http://marc.info/?t=119263748000001&r=1&w=2 ) should
be updated so that any constructor returning NULL is considered a bug,
no matter what the ini settings are.

I would prefer deciding that returning null is the standard way for a constructor to inform the PHP core that the object creation failed (for any reason). This would be trapped by the core and cause a catchable fatal error.

This is more compatible with the 'exception-less' behavior of the core in general. The constructor is still free to throw an exception but this wouldn't be encouraged nor the standard way to inform the core that an error occurred.

It also allows for minimal code changes. This could be split in two steps. First, the core is modified to trap null return values from constructors. Then, class constructors who did implement another mechanism can be modified one by one. I think that it is also better for PECL extensions as, if we don't introduce the check in the core, it is just a convention that extension developers should follow. BC break would be minimal too (would only impact for few programs who had ways to manage null return values from constructors).

Constructors give warning, but are then in an unusable state

Same, should return null and not create object.

Constructor gives error

Should return null, and the core would raise the error. No BC break here (except loosing a possible explanation in error message, but is it a BC break ?)

These changes would need an RFC to be approved I presume. However
doing the RFC would probably require making a patch that implements
most, if not all, of these changes.

IMO, you should write the RFC, as it would be quite fast (even if you forget to list some classes). Please mention at least your option and mine.

On such a subject, I wouldn't implement anything before approval, even a non-official one. We don't need a POC here, as everybody clearly sees the impact the change would have.

Cheers

François

9 years ago by Andrea Faulds — view source

unread

Hi François,

I would prefer deciding that returning null is the standard way for a constructor to inform the PHP core that the object creation failed (for any reason). This would be trapped by the core and cause a catchable fatal error.

This is more compatible with the 'exception-less' behavior of the core in general. The constructor is still free to throw an exception but this wouldn't be encouraged nor the standard way to inform the core that an error occurred.

I don’t really agree here. For some reason we have this tradition of not using exceptions for “procedural” stuff. That sort of makes sense for functions, but classes are “OOP” and therefore I don’t see a good reason why they shouldn’t throw exceptions. Exceptions and OOP (and namespaces, even) should not be the domain of userland exclusively, they are also the domain of internals/core. If you really hate exceptions, I’d say you should just use one of those ridiculous “procedural” alternative functions (that are just methods in disguise) that we went to the bother of adding.

By the way, it irks me (and many others) that PDO doesn’t throw exceptions by default. It really ought to, it’s an OOP API, I’d love to see that changed in PHP 7.

Thanks.

--
Andrea Faulds
http://ajf.me/

9 years ago by Rowan Collins — view source

unread

Hi François,

On 17 Jan 2015, at 15:37, François Laupretre francois@tekwire.net
wrote:

I would prefer deciding that returning null is the standard way for a
constructor to inform the PHP core that the object creation failed (for any
reason). This would be trapped by the core and cause a catchable fatal
error.

This is more compatible with the 'exception-less' behavior of the core
in general. The constructor is still free to throw an exception but this
wouldn't be encouraged nor the standard way to inform the core that an
error occurred.

I don’t really agree here. For some reason we have this tradition of not
using exceptions for “procedural” stuff. That sort of makes sense for
functions, but classes are “OOP” and therefore I don’t see a good reason
why they shouldn’t throw exceptions. Exceptions and OOP (and namespaces,
even) should not be the domain of userland exclusively, they are also the
domain of internals/core. If you really hate exceptions, I’d say you should
just use one of those ridiculous “procedural” alternative functions (that
are just methods in disguise) that we went to the bother of adding.

By the way, it irks me (and many others) that PDO doesn’t throw exceptions
by default. It really ought to, it’s an OOP API, I’d love to see that
changed in PHP 7.

The difference is that AFAIK all the exceptions returned by OOP extensions
right now are of classes specific to that extension, whereas this would be
a global engine-thrown exception. We'd have to decide on the class of the
exception, its position in a class hierarchy, etc. I'm personally in favour
of the core using exceptions more, but only if done properly (I don't like
the idea of a single non-semantic EngineException as previously proposed,
except as the base class for a proper hierarchy).

I'm also in favour of extension objects behaving more like userland ones,
which means that a constructor should either give an instance of the named
class or error (if you need something more flexible, build a factory
instead). If extensions want to throw their own exceptions from
constructors, that's fine, obviously, but it would be more consistent right
now if the generic "extension failed to create the right thing" catch
issued an error like for other "unexpected condition" scenarios.

--
Rowan Collins
[IMSoP]

9 years ago by Dan Ackroyd — view source

unread

The difference is that AFAIK all the exceptions returned by OOP extensions right now are of classes specific to that extension, whereas this would be a global engine-thrown exception.

Sorry - I wasn't clear. I didn't mean they should throw generic
exceptions; they definitely should throw exceptions specific to the
code being called. To a very large part these exceptions already
exist:

The intl extension has: http://php.net/manual/en/class.intlexception.php
The reflection extension has:
http://php.net/manual/en/class.reflectionexception.php

For the intl extension, these are already in use in some places, if
the user has set "intl.use_exceptions".

This is why I think most, if not all, of the work would need to be
done before an RFC could be voted on - it's one of those cases where
people could disagree with details of the RFC, even if they agreed in
the general idea.

cheers
Dan

9 years ago by francois@tekwire.net — view source

unread

De : Andrea Faulds [mailto:ajf@ajf.me]

I don’t really agree here. For some reason we have this tradition of not using
exceptions for “procedural” stuff. That sort of makes sense for functions, but
classes are “OOP” and therefore I don’t see a good reason why they
shouldn’t throw exceptions. Exceptions and OOP (and namespaces, even)
should not be the domain of userland exclusively, they are also the domain
of internals/core. If you really hate exceptions, I’d say you should just use
one of those ridiculous “procedural” alternative functions (that are just
methods in disguise) that we went to the bother of adding.

I agree. I just remembered that, in in the past, every proposal which included exceptions in the core were discarded. I also would prefer exceptions to this old error system. If minds (or people) have changed, that's a very good thing.

François

9 years ago by Tony Marston — view source

unread

"Andrea Faulds" wrote in message
news:23490588-0131-4B0F-A7AA-C9C8C766626C@ajf.me...

Hi François,

On 17 Jan 2015, at 15:37, François Laupretre francois@tekwire.net
wrote:

I would prefer deciding that returning null is the standard way for a
constructor to inform the PHP core that the object creation failed (for
any reason). This would be trapped by the core and cause a catchable
fatal error.

This is more compatible with the 'exception-less' behavior of the core in
general. The constructor is still free to throw an exception but this
wouldn't be encouraged nor the standard way to inform the core that an
error occurred.

I don’t really agree here. For some reason we have this tradition of not
using exceptions for “procedural” stuff. That sort of makes sense for
functions, but classes are “OOP” and therefore I don’t see a good reason
why they shouldn’t throw exceptions.

You are forgetting one minor thing - an object may be instantiated from a
piece of procedural code. In fact the very first object can only be
instantiated from procedural code. The idea that every piece of code that
uses "new" will have to be amended to be in a try...catch block is a huge
amount of work that does not solve any problem (except to satisfy the
delicate sensibilities of the OO purists).

Exceptions and OOP (and namespaces, even) should not be the domain of
userland exclusively, they are also the domain of internals/core. If you
really hate exceptions, I’d say you should just use one of those ridiculous
“procedural” alternative functions (that are just methods in disguise) that
we went to the bother of adding.

If you really want a clever way of introducing exceptions into PHP without
breaking huge amounts of existing code then how about this idea - change the
language to detect if a function/method is being called within a try...catch
block or not. If it is then throw an exception, but if it is not then use
the old error handler. In this way the use of exceptions would not be forced
on the developer and existing code would not break. This would me more work
for the core developers, but no broken code in userland.

By the way, it irks me (and many others) that PDO doesn’t throw exceptions
by default. It really ought to, it’s an OOP API, I’d love to see that
changed in PHP 7.

Thanks.

--
Andrea Faulds
http://ajf.me/

--
Tony Marston

9 years ago by Rowan Collins — view source

unread

"Andrea Faulds" wrote in message news:23490588-0131-4B0F-A7AA-
C9C8C766626C@ajf.me...

Hi François,

On 17 Jan 2015, at 15:37, François Laupretre francois@tekwire.net

wrote:

I would prefer deciding that returning null is the standard way for a
constructor to inform the PHP core that the object creation failed (for any
reason). This would be trapped by the core and cause a catchable fatal
error.

This is more compatible with the 'exception-less' behavior of the core
in general. The constructor is still free to throw an exception but this
wouldn't be encouraged nor the standard way to inform the core that an
error occurred.

I don’t really agree here. For some reason we have this tradition of not
using exceptions for “procedural” stuff. That sort of makes sense for
functions, but classes are “OOP” and therefore I don’t see a good reason
why they shouldn’t throw exceptions.

You are forgetting one minor thing - an object may be instantiated from a
piece of procedural code. In fact the very first object can only be
instantiated from procedural code.

I'm not sure what you mean by the second sentence here; which is the "very
first object"?

The idea that every piece of code that uses "new" will have to be amended
to be in a try...catch block is a huge amount of work that does not solve
any problem (except to satisfy the delicate sensibilities of the OO
purists).

Bear in mind that there are costs to users writing new code as well as
costs to those maintaining existing ones. When writing code from scratch,
it's actually much easier to write code with catch/finally than to check
for non-object returns before using every object. The errors for uncaught
exceptions are generally much easier for a new user to understand than the
errors for accessing a method on what turns out to be a null value not an
object.

Exceptions and OOP (and namespaces, even) should not be the domain of

userland exclusively, they are also the domain of internals/core. If you
really hate exceptions, I’d say you should just use one of those ridiculous
“procedural” alternative functions (that are just methods in disguise) that
we went to the bother of adding.

If you really want a clever way of introducing exceptions into PHP without
breaking huge amounts of existing code then how about this idea - change
the language to detect if a function/method is being called within a
try...catch block or not. If it is then throw an exception, but if it is
not then use the old error handler. In this way the use of exceptions would
not be forced on the developer and existing code would not break. This
would me more work for the core developers, but no broken code in userland.

Unfortunately, that wouldn't be as useful as it sounds. Consider this code:

try { somefunc($foo); }
catch ( InvalidArgumentException $e ) { ... }

If somefunc() internally uses PDO, should it automatically switch to
exception-throwing mode, because a try-block is in the stack? Or should it
hunt in advance for matching catch() clauses, which would probably be
extremely slow? And if the user lazily writes catch(Exception $e) around a
top-level function, suddenly their whole application behaves differently,
because their inline error handling suddenly stops working?

Unfortunately, exceptions require a specific way of thinking, at least in
the area of code where they occur, because they don't neatly reduce to
equivalent linear code.

--
Rowan Collins
[IMSoP]

9 years ago by Tony Marston — view source

unread

"Rowan Collins" wrote in message
news:CALKiJKqpk7X0yn0d_PWOKU1S7m156dF7865aLCP5dLF=q39uWg@mail.gmail.com...

"Andrea Faulds" wrote in message news:23490588-0131-4B0F-A7AA-
C9C8C766626C@ajf.me...

<<snip>>

You are forgetting one minor thing - an object may be instantiated from a
piece of procedural code. In fact the very first object can only be
instantiated from procedural code.

I'm not sure what you mean by the second sentence here; which is the "very
first object"?

Before you can use an object you must instantiate it using "$object = new
<classname>". For the very first object this line can only be performed in
procedural code. It cannot be perform within the first object as the first
object must be instantiated externally.

--
Tony Marston

9 years ago by Rowan Collins — view source

unread

Tony Marston wrote on 19/01/2015 09:54:

You are forgetting one minor thing - an object may be instantiated
from a
piece of procedural code. In fact the very first object can only be
instantiated from procedural code.

I'm not sure what you mean by the second sentence here; which is the
"very
first object"?

Before you can use an object you must instantiate it using "$object =
new <classname>". For the very first object this line can only be
performed in procedural code. It cannot be perform within the first
object as the first object must be instantiated externally.

Ah, I see, yes. I don't think that's what was meant by "procedural code"
in this case, though. We were talking about extensions which either
represent all their functionality procedurally (as with file I/O, for
instance), or which include a procedural wrapper for those not
comfortable with OO (such as mysqli). The point being that these
functions are implemented in a different style, including their
error-handling conventions.

Regards,

Rowan Collins
[IMSoP]

9 years ago by Marco Pivetta — view source

unread

On 17 January 2015 at 16:37, François Laupretre francois@tekwire.net
wrote:

De : Dan Ackroyd [mailto:danack@basereality.com]

This is pretty horrible and should be fixed by making sure that
constructors either return an object or throw an exception.
Additionally the exception policy for core (that was previously
discussed here: http://marc.info/?t=119263748000001&r=1&w=2 ) should
be updated so that any constructor returning NULL is considered a bug,
no matter what the ini settings are.

I would prefer deciding that returning null is the standard way for a
constructor to inform the PHP core that the object creation failed (for any
reason). This would be trapped by the core and cause a catchable fatal
error.

A constructor that fails is a hard failure (factory method failed to
produce the expected value), and is an exceptional case that can or cannot
be handled (via catch).
It's not just a failed operation (expected to eventually fail), but
something really went wrong, badly.

An exception fits perfectly in this case.

Additionally, it makes no sense to have inconsistent behavior between
internal classes and userland classes: it is currently impossible for a
userland class to have the new operator producing an object that is not
an instance of the class after that operator.

It feels weird, from a user perspective, to have new producing non-object
values.

The general feeling when dealing with cases such as PDO's constructor is
"WAT?!".

Therefore gogogo Danack!

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

9 years ago by Stanislav Malyshev — view source

unread

Hi!

A constructor that fails is a hard failure (factory method failed to
produce the expected value), and is an exceptional case that can or cannot
be handled (via catch).
It's not just a failed operation (expected to eventually fail), but
something really went wrong, badly.

Why it is different from any other failure? I.e. fopen("doesnotexist",
"r") produces false and it's not "hard failure". But if files were
objects, new File("doesnotexist", File::READONLY) suddenly becomes "hard
failure"? How the latter is harder than the former? I would say it's
exactly the same. If you say "new File" can only throw, you should also
say fopen() should only throw. If fopen can return error value, so can
File::open(), so can new File.
The difference would be if File object would be built so that "new File"
should never fail, and opening would be done with "$file->open()" and
not with the ctor. But most of the object APIs in PHP are not written
this way - for these APIs, ctor failure is not something exceptional
more than failing to open file is exceptional.

Additionally, it makes no sense to have inconsistent behavior between
internal classes and userland classes: it is currently impossible for a
userland class to have the new operator producing an object that is not
an instance of the class after that operator.

That can be changed if needed. This is just a missing feature.

--
Stas Malyshev
smalyshev@gmail.com

9 years ago by Peter Cowburn — view source

unread

Hi!

A constructor that fails is a hard failure (factory method failed to
produce the expected value), and is an exceptional case that can or
cannot
be handled (via catch).
It's not just a failed operation (expected to eventually fail), but
something really went wrong, badly.

Why it is different from any other failure? I.e. fopen("doesnotexist",
"r") produces false and it's not "hard failure". But if files were
objects, new File("doesnotexist", File::READONLY) suddenly becomes "hard
failure"? How the latter is harder than the former? I would say it's
exactly the same. If you say "new File" can only throw, you should also
say fopen() should only throw. If fopen can return error value, so can
File::open(), so can new File.

Excerpt from SplFileObject's manual page [1]:

Throws a RuntimeException if the filename cannot be opened.

[1] http://php.net/splfileobject.construct

The difference would be if File object would be built so that "new File"
should never fail, and opening would be done with "$file->open()" and
not with the ctor. But most of the object APIs in PHP are not written
this way - for these APIs, ctor failure is not something exceptional
more than failing to open file is exceptional.

Additionally, it makes no sense to have inconsistent behavior between
internal classes and userland classes: it is currently impossible for a
userland class to have the new operator producing an object that is not
an instance of the class after that operator.

That can be changed if needed. This is just a missing feature.

--
Stas Malyshev
smalyshev@gmail.com

9 years ago by Sebastian Bergmann — view source

unread

Am 17.01.2015 um 15:02 schrieb Dan Ackroyd:

class constructors

<nitpicking> PHP does not have class constructors (static method automatically invoked when a class is initialized). It only supports object constructors (method automatically invoked when an object is created). </nitpicking>

Several classes in PHP return null when there is a problem in the
parameters passed to their constructor e.g.

[...]

Several constructors check the parameters that they are given.....and
then just give a warning when they are not acceptable. e.g.

[...]

Some constructors check the parameters they are given, and then emit
either a 'Catchable fatal error' error or other error e.g.

Classes implemented in C / provided by PHP must not have any of
the above issues. +1 for fixing them.

9 years ago by Matteo Beccati — view source

unread

Hi,

Several classes in PHP return null when there is a problem in the
parameters passed to their constructor e.g.

[...]

Several constructors check the parameters that they are given.....and
then just give a warning when they are not acceptable. e.g.

[...]

Some constructors check the parameters they are given, and then emit
either a 'Catchable fatal error' error or other error e.g.

Classes implemented in C / provided by PHP must not have any of
the above issues. +1 for fixing them.

+1 here as well. Sounds like material for a new RFC.

Cheers

Matteo Beccati

Development & Consulting - http://www.beccati.com/

9 years ago by Stanislav Malyshev — view source

unread

Hi!

This is pretty horrible and should be fixed by making sure that

I don't see why it's so horrible. In fact, handling null is easier than
handling exception (and faster too, probably). Many functions that try
to create objects return null or false when unable to. Of course, we
could change it by introducing exceptions to core, but I don't see why
do it peacemeal then.

constructors either return an object or throw an exception.
Additionally the exception policy for core (that was previously
discussed here: http://marc.info/?t=119263748000001&r=1&w=2 ) should
be updated so that any constructor returning NULL is considered a bug,
no matter what the ini settings are. To be clear, procedural code
should behave as before, with users expected to check for errors.
This would be a BC break for people who are handling the constructor
returning null currently, as they would need to wrap that code with an
try/catch block.

That means the same failure of collator construction now needs to be
handled in two different ways.

Constructors give warning, but are then in an unusable state

Several constructors check the parameters that they are given.....and
then just give a warning when they are not acceptable. e.g.

These should be fixed to either return null or throw exception, but the
behavior should be unified for everybody.

Again, this should be fixed by changing the constructors to throw an
exception if the input parameters are not acceptable.

The next question would be - if the ctors throw an exceptions, why
factory methods shouldn't? They are doing the same work essentially.

Constructor gives error

Some constructors check the parameters they are given, and then emit
either a 'Catchable fatal error' error or other error e.g.

This is even bigger can of worms. If we wrote PHP today from scratch,
catchable fatal errors would be exceptions. But they are not, so we need
to deal with it - by either leaving them as is, or converting them all
together.

<?php
$foo = new IntlGregorianCalendar(new StdClass);
//Output: Catchable fatal error: Object of class stdClass could not be
converted to string in..

This is not property of the ctor, it's property of parameter handling.
I.e. if you do it for IntlGregorianCalendar, you should do it for every
class that gets incompatible parameters. And probably user-space funcs
too since there's no reason for them to behave differently. I actually
wouldn't mind that, but this is yet bigger can of worms as it requires
refactoring a lot of assumptions on how parameters are parsed.

i) Can anyone see a big hurdle in fixing these behaviours, other than
it being a BC break for people who are currently relying on these
behaviours ?

Yes. The first hurdle is to understand which of these behaviors need
fixing, and how, which you seem to dismiss as obvious but it's really
not. The second is to do it in consistent manner so it doesn't again
come out as part of the functions do one thing, another part another,
and third part something else different.

ii) Are there any other bad behaviours that people are aware of that
ought to be fixed at a major version?

I think we have enough for now to discuss here, so if we want do discuss
other behaviors better to open separate thread.

--
Stas Malyshev
smalyshev@gmail.com

9 years ago by Ferenc Kovacs — view source

unread

On Mon, Jan 19, 2015 at 6:41 AM, Stanislav Malyshev smalyshev@gmail.com
wrote:

Hi!

This is pretty horrible and should be fixed by making sure that

I don't see why it's so horrible. In fact, handling null is easier than
handling exception (and faster too, probably). Many functions that try
to create objects return null or false when unable to. Of course, we
could change it by introducing exceptions to core, but I don't see why
do it peacemeal then.

constructors either return an object or throw an exception.
Additionally the exception policy for core (that was previously
discussed here: http://marc.info/?t=119263748000001&r=1&w=2 ) should
be updated so that any constructor returning NULL is considered a bug,
no matter what the ini settings are. To be clear, procedural code
should behave as before, with users expected to check for errors.
This would be a BC break for people who are handling the constructor
returning null currently, as they would need to wrap that code with an
try/catch block.

That means the same failure of collator construction now needs to be
handled in two different ways.

Constructors give warning, but are then in an unusable state

Several constructors check the parameters that they are given.....and
then just give a warning when they are not acceptable. e.g.

These should be fixed to either return null or throw exception, but the
behavior should be unified for everybody.

Again, this should be fixed by changing the constructors to throw an
exception if the input parameters are not acceptable.

The next question would be - if the ctors throw an exceptions, why
factory methods shouldn't? They are doing the same work essentially.

Constructor gives error

Some constructors check the parameters they are given, and then emit
either a 'Catchable fatal error' error or other error e.g.

This is even bigger can of worms. If we wrote PHP today from scratch,
catchable fatal errors would be exceptions. But they are not, so we need
to deal with it - by either leaving them as is, or converting them all
together.

<?php
$foo = new IntlGregorianCalendar(new StdClass);
//Output: Catchable fatal error: Object of class stdClass could not be
converted to string in..

This is not property of the ctor, it's property of parameter handling.
I.e. if you do it for IntlGregorianCalendar, you should do it for every
class that gets incompatible parameters. And probably user-space funcs
too since there's no reason for them to behave differently. I actually
wouldn't mind that, but this is yet bigger can of worms as it requires
refactoring a lot of assumptions on how parameters are parsed.

i) Can anyone see a big hurdle in fixing these behaviours, other than
it being a BC break for people who are currently relying on these
behaviours ?

Yes. The first hurdle is to understand which of these behaviors need
fixing, and how, which you seem to dismiss as obvious but it's really
not. The second is to do it in consistent manner so it doesn't again
come out as part of the functions do one thing, another part another,
and third part something else different.

ii) Are there any other bad behaviours that people are aware of that
ought to be fixed at a major version?

I think we have enough for now to discuss here, so if we want do discuss
other behaviors better to open separate thread.

--
Stas Malyshev
smalyshev@gmail.com

--

Hi,

AFAIK (based on
internals@lists.php.net/msg43237.html" rel="nofollow" target="_blank">http://www.mail-archive.com/internals@lists.php.net/msg43237.html and other
discussions) errors in constructors should throw exceptions (even for
classes in core).

--
Ferenc Kovács
@Tyr43l - http://tyrael.hu

9 years ago by Michael Wallner — view source

unread

On Mon, Jan 19, 2015 at 6:41 AM, Stanislav Malyshev smalyshev@gmail.com
wrote:

Hi!

This is pretty horrible and should be fixed by making sure that

I don't see why it's so horrible. In fact, handling null is easier than
handling exception (and faster too, probably). Many functions that try
to create objects return null or false when unable to. Of course, we
could change it by introducing exceptions to core, but I don't see why
do it peacemeal then.

constructors either return an object or throw an exception.
Additionally the exception policy for core (that was previously
discussed here: http://marc.info/?t=119263748000001&r=1&w=2 ) should
be updated so that any constructor returning NULL is considered a bug,
no matter what the ini settings are. To be clear, procedural code
should behave as before, with users expected to check for errors.
This would be a BC break for people who are handling the constructor
returning null currently, as they would need to wrap that code with an
try/catch block.

That means the same failure of collator construction now needs to be
handled in two different ways.

Constructors give warning, but are then in an unusable state

Several constructors check the parameters that they are given.....and
then just give a warning when they are not acceptable. e.g.

These should be fixed to either return null or throw exception, but the
behavior should be unified for everybody.

Again, this should be fixed by changing the constructors to throw an
exception if the input parameters are not acceptable.

The next question would be - if the ctors throw an exceptions, why
factory methods shouldn't? They are doing the same work essentially.

Constructor gives error

Some constructors check the parameters they are given, and then emit
either a 'Catchable fatal error' error or other error e.g.

This is even bigger can of worms. If we wrote PHP today from scratch,
catchable fatal errors would be exceptions. But they are not, so we need
to deal with it - by either leaving them as is, or converting them all
together.

<?php
$foo = new IntlGregorianCalendar(new StdClass);
//Output: Catchable fatal error: Object of class stdClass could not be
converted to string in..

This is not property of the ctor, it's property of parameter handling.
I.e. if you do it for IntlGregorianCalendar, you should do it for every
class that gets incompatible parameters. And probably user-space funcs
too since there's no reason for them to behave differently. I actually
wouldn't mind that, but this is yet bigger can of worms as it requires
refactoring a lot of assumptions on how parameters are parsed.

i) Can anyone see a big hurdle in fixing these behaviours, other than
it being a BC break for people who are currently relying on these
behaviours ?

Yes. The first hurdle is to understand which of these behaviors need
fixing, and how, which you seem to dismiss as obvious but it's really
not. The second is to do it in consistent manner so it doesn't again
come out as part of the functions do one thing, another part another,
and third part something else different.

ii) Are there any other bad behaviours that people are aware of that
ought to be fixed at a major version?

I think we have enough for now to discuss here, so if we want do discuss
other behaviors better to open separate thread.

--
Stas Malyshev
smalyshev@gmail.com

--

Hi,

AFAIK (based on
internals@lists.php.net/msg43237.html" rel="nofollow" target="_blank">http://www.mail-archive.com/internals@lists.php.net/msg43237.html and
other
discussions) errors in constructors should throw exceptions (even for
classes in core).

Definitely my take on this, too. If userland constructors cannot return
anything than an instance of the class, why should internal?

9 years ago by Ferenc Kovacs — view source

unread

On Mon, Jan 19, 2015 at 6:41 AM, Stanislav Malyshev <smalyshev@gmail.com

wrote:

Hi!

This is pretty horrible and should be fixed by making sure that

I don't see why it's so horrible. In fact, handling null is easier than
handling exception (and faster too, probably). Many functions that try
to create objects return null or false when unable to. Of course, we
could change it by introducing exceptions to core, but I don't see why
do it peacemeal then.

constructors either return an object or throw an exception.
Additionally the exception policy for core (that was previously
discussed here: http://marc.info/?t=119263748000001&r=1&w=2 ) should
be updated so that any constructor returning NULL is considered a
bug,
no matter what the ini settings are. To be clear, procedural code
should behave as before, with users expected to check for errors.
This would be a BC break for people who are handling the constructor
returning null currently, as they would need to wrap that code with
an
try/catch block.

That means the same failure of collator construction now needs to be
handled in two different ways.

Constructors give warning, but are then in an unusable state

Several constructors check the parameters that they are given.....and
then just give a warning when they are not acceptable. e.g.

These should be fixed to either return null or throw exception, but the
behavior should be unified for everybody.

Again, this should be fixed by changing the constructors to throw an
exception if the input parameters are not acceptable.

The next question would be - if the ctors throw an exceptions, why
factory methods shouldn't? They are doing the same work essentially.

Constructor gives error

Some constructors check the parameters they are given, and then emit
either a 'Catchable fatal error' error or other error e.g.

This is even bigger can of worms. If we wrote PHP today from scratch,
catchable fatal errors would be exceptions. But they are not, so we
need
to deal with it - by either leaving them as is, or converting them all
together.

<?php
$foo = new IntlGregorianCalendar(new StdClass);
//Output: Catchable fatal error: Object of class stdClass could not
be
converted to string in..

This is not property of the ctor, it's property of parameter handling.
I.e. if you do it for IntlGregorianCalendar, you should do it for every
class that gets incompatible parameters. And probably user-space funcs
too since there's no reason for them to behave differently. I actually
wouldn't mind that, but this is yet bigger can of worms as it requires
refactoring a lot of assumptions on how parameters are parsed.

i) Can anyone see a big hurdle in fixing these behaviours, other than
it being a BC break for people who are currently relying on these
behaviours ?

Yes. The first hurdle is to understand which of these behaviors need
fixing, and how, which you seem to dismiss as obvious but it's really
not. The second is to do it in consistent manner so it doesn't again
come out as part of the functions do one thing, another part another,
and third part something else different.

ii) Are there any other bad behaviours that people are aware of that
ought to be fixed at a major version?

I think we have enough for now to discuss here, so if we want do
discuss
other behaviors better to open separate thread.

--
Stas Malyshev
smalyshev@gmail.com

--

Hi,

AFAIK (based on
internals@lists.php.net/msg43237.html" rel="nofollow" target="_blank">http://www.mail-archive.com/internals@lists.php.net/msg43237.html and
other
discussions) errors in constructors should throw exceptions (even for
classes in core).

Definitely my take on this, too. If userland constructors cannot return
anything than an instance of the class, why should internal?

yep, ofc. this means that there are a couple of class method->function
aliases which have to be changed/refactored, so calling the procedural
counterpart won't throw exceptions, but thats shouldn't be a problem.

--
Ferenc Kovács
@Tyr43l - http://tyrael.hu