Optimize MutableCreationOptionsInterface capability

[noopable]

This enables Not only the factories but also the abstract factories implementing MutableCreationOptionsInterface to be able to get creation options.

[bakura10]

Please factorize all those if into one single if (or at least group them in two if if you prefer) as you break in all cases.

[noopable]

I'm not good at English, so I'm sorry if I misunderstood you.
The method (createServiceViaCallback) is a protected method. It is only called from 'createFromFactory' and 'createFromAbstractFactory'. And it is a common gateway to create an object by the *factory. These call it by a callback array. And the creationOptions is only set at AbstractPluginManager::get if a client needs to set constructor options.
Thus I think this patch(or hotfix) will not cover all cases but covering the known cases.
My first commit is a step-by-step approach getting ability to get the creationOptions with a determinate minimum side effect.
My second commit enables a callable object to get creationOptions.
Are there any other necessary things?

[bakura10]

What I told you is that all your if break, so you should factorize the if. For instance, replace this:

if (A) {
    break;
}

if (B) {
    break;
}

by:

if (A || B) {
    break;
}

it's more efficient and more compact :).

[noopable]

Oh I'm sorry for misunderstanding you.
But is it a code standard problem or a project rule that you pointed out ?
Or too ugly?
I usually chunk code-blocks with the aim of preserving the condition intent.
Each if-line is

• Is it factory object or not?
• Does the interface want to get creationOptions?
• The client want to use creation option or not ?
• Is it valid the specified creationOptions ?

The habit of these is efficient to debug with step-in tools .And more easy to check lines, easy to edit.

if (!isset($factory) || !is_object($factory)) {
    break;
}
if (!$factory instanceof MutableCreationOptionsInterface) {
    break;
}
if (null === $this->creationOptions) {
    break;
}
if (is_array($this->creationOptions) && empty($this->creationOptions)) {
    break;
}

But My habit is trivial matter.
I can remake these if-clause to the one-line.
Is that better?

if (is_object($callable)) {
    $factory = $callable;
} elseif (is_array($callable)) {
    $factory = reset($callable);
}
if (isset($factory) && is_object($factory) && ($factory instanceof MutableCreationOptionsInterface)
    && (null !== $this->creationOptions) && is_array($this->creationOptions) && !empty($this->creationOptions)){
    $factory->setCreationOptions($this->creationOptions); 
}

[bakura10]

これわすごいね!

Service manager is a component that need to be efficient as it's used nearly everywhere, so if we can avoid some if... it's always good ;-)

[sasezaki]

@bakura10 You did typo! s/これわすごいね!/これはすごいね!/ (´・ω・`)

[bakura10]

Oh yeah... How did I make this mistake, this is the only thing irregular in japanese :D.

[iquabius]

If you're testing $factory against an interface, you can remove is_object(), and you don't need to check if null !== $this->creationOptions because is_array() will return false if it is null.

[noopable]

Thanks all.

I used to check by is_object in php old version but now not necessary. I removed it.

(null !== $this->creationOptions) is used in current
https://github.com/zendframework/zf2/blob/master/library/Zend/ServiceManager/AbstractPluginManager.php#L192
Can I rewrite it on the way?

And so, should I remake the 6 commits to 2 ?

[iquabius]

(null !== $this->creationOptions) is used in current
https://github.com/zendframework/zf2/blob/master/library/Zend/ServiceManager/AbstractPluginManager.php#L192
Can I rewrite it on the way?

That one line is slightly different and I think it is right the way it is.

And so, should I remake the 6 commits to 2 ?

That will be good since it will make the git history cleaner.

[noopable]

I have no idea about this.
Travis saids:

BUILD FAILED

/home/travis/build/zendframework/zf2/build.xml:15: Can't get http://cs.sensiolabs.org/get/php-cs-fixer.phar to
/home/travis/build/zendframework/zf2/php-cs-fixer.phar

[iquabius]

That have been happening for some time, I'm not sure why but you don't need to worry about that, it is just a Travis issue.

[noopable]

Memo: if $factory is not object, 'instanceof' raises fatal error. To do fix it?

[iquabius]

That actually does not happen, at least with php 5.3 and up!

[noopable]

Is that really?
Document saids,

Constants, however, are not allowed. 

http://php.net/manual/en/language.operators.type.php#example-130

I tested it.

> php -v
PHP 5.4.13 (cli) (built: Mar 15 2013 02:07:14)
Copyright (c) 1997-2013 The PHP Group
Zend Engine v2.4.0, Copyright (c) 1998-2013 Zend Technologies
    with Xdebug v2.2.2, Copyright (c) 2002-2013, by Derick Rethans

<?php
var_dump((false instanceof \StdClass));

raises
PHP Fatal error: instanceof expects an object instance, constant given in - on line 2

Please try it.

[iquabius]

you are right, but the code is not using a constant, it is using a variable ($factory), so there's no way of this to happen.

[noopable]

Many thanks.
I was anxious about it . Now I see.