Impossible do define "null" value #79

Closed
mnapoli opened this Issue Jun 19, 2013 · 7 comments

Comments

Projects
None yet
2 participants
@mnapoli
Member

mnapoli commented Jun 19, 2013

public function set($name, $value = null)

Impossible to define explicitly a null value:

$container->set('service', null);

because:

if null, returns a ClassDefinitionHelper

so it will create a ClassDefinition for the entry name ("service" in the example).


One solution would be to check the numbers of arguments given: if $value was given and specified explicitly to null, then we define a null value.

Another would be to return a more generic helper that would allow:

$container->set('service')->value(null);

but that seems complex because of how it currently works with the ClassDefinitionHelper.

Finally, by adding a NullValue class:

$container->set('service', new NullValue());

@ghost ghost assigned mnapoli Jun 19, 2013

mnapoli added a commit that referenced this issue Jun 23, 2013

@mnapoli

This comment has been minimized.

Show comment
Hide comment
@mnapoli

mnapoli Jun 23, 2013

Member

Fixed.

// Will set a null value
$container->set('foo', null);

// Will return a ClassDefinitionHelper for inline definition
$container->set('foo')->addParameter(/* ... */);

As a consequence:

// Will not do anything as such
$container->set('foo');

// Will throw an exception (because set returns null)
$container->set('foo', null)->addParameter(/* ... */);
Member

mnapoli commented Jun 23, 2013

Fixed.

// Will set a null value
$container->set('foo', null);

// Will return a ClassDefinitionHelper for inline definition
$container->set('foo')->addParameter(/* ... */);

As a consequence:

// Will not do anything as such
$container->set('foo');

// Will throw an exception (because set returns null)
$container->set('foo', null)->addParameter(/* ... */);

@mnapoli mnapoli closed this Jun 23, 2013

@dsummersl

This comment has been minimized.

Show comment
Hide comment
@dsummersl

dsummersl Aug 6, 2013

How does this work in YAML? I have tried variations of:

My\Implementation:
  constructor:
    param: null

and

My\Implementation:
  constructor:
    param: NullParam

and

nullparam: null
My\Implementation:
  constructor:
    param: nullparam

How does this work in YAML? I have tried variations of:

My\Implementation:
  constructor:
    param: null

and

My\Implementation:
  constructor:
    param: NullParam

and

nullparam: null
My\Implementation:
  constructor:
    param: nullparam
@mnapoli

This comment has been minimized.

Show comment
Hide comment
@mnapoli

mnapoli Aug 6, 2013

Member

In the YAML definitions, parameters are always references to other entries, so as you guessed:

My\Implementation:
  constructor:
    param: null

will look for an entry for the name "null" in the container.


I just learned that YAML supports null value, and in different forms. Apparently, you can use any of this:

nullparam: null
nullparam: ~
nullparam:

(see also http://yaml.org/type/null.html or symfony/symfony#7274)

The question now is: does PHP-DI supports it on its end. I must confess I haven't tried it, and there are no tests for this. Given your last suggestion is valid YAML, if you have tried it and it doesn't work, then it's a bug.

Could you confirm that any of the 3 solutions above don't work?

Member

mnapoli commented Aug 6, 2013

In the YAML definitions, parameters are always references to other entries, so as you guessed:

My\Implementation:
  constructor:
    param: null

will look for an entry for the name "null" in the container.


I just learned that YAML supports null value, and in different forms. Apparently, you can use any of this:

nullparam: null
nullparam: ~
nullparam:

(see also http://yaml.org/type/null.html or symfony/symfony#7274)

The question now is: does PHP-DI supports it on its end. I must confess I haven't tried it, and there are no tests for this. Given your last suggestion is valid YAML, if you have tried it and it doesn't work, then it's a bug.

Could you confirm that any of the 3 solutions above don't work?

@dsummersl

This comment has been minimized.

Show comment
Hide comment
@dsummersl

dsummersl Aug 7, 2013

I tried all three. I get the following exception in all three cases:

ReflectionException: Class nullparam does not exist in /opt/site/vendor/mnapoli/php-di/src/DI/Factory.php on line 48

I tried all three. I get the following exception in all three cases:

ReflectionException: Class nullparam does not exist in /opt/site/vendor/mnapoli/php-di/src/DI/Factory.php on line 48
@mnapoli

This comment has been minimized.

Show comment
Hide comment
@mnapoli

mnapoli Aug 7, 2013

Member

Thanks, I have created a new issue for this: #95

Member

mnapoli commented Aug 7, 2013

Thanks, I have created a new issue for this: #95

@dsummersl

This comment has been minimized.

Show comment
Hide comment
@dsummersl

dsummersl Aug 7, 2013

Thanks! If this is something that you dno't have time for I can also devote a little time to debugging this today...

Thanks! If this is something that you dno't have time for I can also devote a little time to debugging this today...

@mnapoli

This comment has been minimized.

Show comment
Hide comment
@mnapoli

mnapoli Aug 7, 2013

Member

thanks, feel free to submit a pull request. If you have time only to write tests to reproduce it that's good too. FYI I may not have a lot of time to look at it in the next days.

Member

mnapoli commented Aug 7, 2013

thanks, feel free to submit a pull request. If you have time only to write tests to reproduce it that's good too. FYI I may not have a lot of time to look at it in the next days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment