Open jubianchi opened 9 years ago
I'm implementing the feature and I don't know what to choose between:
1.
$testedClassName = $this->getTestedClassName();
$this->instanciateTestedClassWith(function($arg1, $arg2 = null) use ($testedClassName) {
return new $testedClassName($arg1, $arg2);
});
2.
$this->instanciateTestedClassWith(function($testedClassName, $arg1, $arg2 = null) {
return new $testedClassName($arg1, $arg2);
});
3.
$this->instanciateTestedClassWith(function(\reflectionClass $testedClass, $arg1, $arg2 = null) {
return $testedClass->newInstanceArgs(array($arg1, $arg2));
});
The 1.
forces the user to set a temporary variable holding the tested class name and use it in the closure. So I wrote the 2.
which automatically passes the tested class name as the first argument of the factory closure. And then, I came up with the third solution which seems more flexible to me.
if we only consider php >= 5.3 && < 5.6
I would say that the 2.
is the best. Now if we consider PHP >= 5.6
the 3.
seems better, for example it will allow things like
$this->instanciateTestedClassWith(function(\reflectionClass $testedClass, $arg1 = null, ...$args) {
array_unshift($args, $arg1 ?: 'foobar');
return $testedClass->newInstanceArgs($args);
});
@atoum/all let me know what you think :)
3 sounds pretty good.
I vote for 2 which sounds simple to use / understand
@esion in fact, you are right. As we discussed together, the 2.
still lets the user do:
$this->instanciateTestedClassWith(function($testedClassName, $arg1 = null, ...$args) {
array_unshift($args, $arg1 ?: 'foobar');
return new $testedClassName(...$args);
});
So now I would say 2.
is better for both php >= 5.3 && < 5.6
and >= 5.6
2 looks good for me too.
My initial reaction was to find solution 2 easier to understand, but the @jubianchi last argument finally convinced me for the solution 3
ping ?
up
let drop it?
@vonglasow reported a problem using
newTestedInstance
/newInstance
on PHP classes for which the reflection is not correct:This test will produce an error in the
testBar
method only:SplplFileObject::__construct(test.php): failed to open stream: Invalid argument
. Why ? Because the reflection on\SplFileObject
is wrong: https://3v4l.org/YP4oRAs a workaround, we could provide some kind of configurable factory for the
newTestedInstance
/newInstance
methods. Here is an example: