[BUG]: Default value ignored in Phalcon\Config

[wurst-hans]

In Phalcon v4.1.0 it is not possible to use defaultValue in method get of Config component. It only works, when the requested config key does not exist really.

In v4 the Config class extends Collection. When looking at code it seems, that default value is returned only, when key cannot be fetched (i.e., it does not exist). In v3 one can see that default value is returned always, when property is not set. So, in v3 it returns default value even when existing config key value is set to null.

I expect in v4 that a $config->get('somekey', 'fallback'); will return fallback when somekey is not defined or is set to null.

This is the commit, where behavior had been changed.

[niden]

I see where the issue is. The question though is what happens if you want to have a null value in a key? If memory serves right that was one of the reasons that behavior changed. The default value should be returned when the key does not exist or cannot be fetched for whatever other reason. If it is null then it should return null.

Personally I see this as a documentation omission not a bug.

[wurst-hans]

I see and it's hard to discuss about this. Sure, internally you can distinguish between a non-existing key and and key that is set to null, but consider these two cases:

$first = $config->get('missing');
$second = $config->get('missing', 'some');

Looking from application perspective $first contains null when key does not exist or even when it exists, but is null.
For $second I get some when key does not exist and even when it is set to some in config already.

So, for both cases I cannot distinguish for which reason I get null or some. In both cases the key can be missing or set to default value already. For outer scopes it does not make sense, because config cannot return back the information that a key is missing really, because it will be default value (in most cases null) then.
It's a pity, that this behavior has been changed silently, because it's complete different from other frameworks now.

Additionally, the definition of null is, that the value is undefined. isset($missing) and isset(null) returns false for not defined $missing and $missing = null too. Also, in scope of database (i.e., MySQL) there is no other definition than null for undefined values.

The dilemma is now, that many people, which have migrated their projects from v3 to v4 might rely on the old behavior, like me.

[niden]

The v3 version allowed you to ignore null values, by always getting the default value if the key did not exist or it was null. As such in the previous version you would never know if a key had a null value because the default would always be returned.

To check if a key exists one has to use has(). That is the safest way to do this.

One can easily extend the Config and simulate the v3.

@Jeckerson thoughts?

[Jeckerson]

As @niden mentioned, if you want to use fallback value in such code: $config->get('somekey', 'fallback'); in v4, you will need to check first if key exists:

$value = $config->has('somekey') ? $config->get('somekey') : 'fallback';

In PHP, isset() does not always guarantee that key exists (for that we use array_key_exists()), because in some cases you will need key with empty value: 0, '', 'null'. And as Phalcon v4 supports only 7.x versions where types begging to be more stricter, that is why there is that changes...

[wurst-hans]

isset() is not empty()! isset(0) returns true which will result in not returning default value but 0. Even an empty string is true. The one, and really only one case that seems to be valid for me: When config value is set to null already, then I cannot distinguish the origin.

I understand your intention. But I believe the most people have such a use case like "get config value and return fallback if empty". If I really want to know, if a config value is not set explicitely, I would have to use has(). This seems more user friendly to me.
To achieve what I want, I have to use $value = $config->get('somekey') ?? 'default'; (or more lax ?:) but not the example of Jeckerson, because this would return null which I don't want to get.

But the decision has been made by Phalcon team. Thanks nevertheless.

BTW: I love you guys for your work, so my disagreement to your decision does not affect my love to Phalcon ;-)

[niden]

@wurst-hans Revisiting this one.

Current behavior:

$data = [
    "one"   => "value",
    "two"   => "",
    "three" => null,
];

$collection = new Collection($data);

// Exists
var_dump($collection->get("one", "fallback"));
// "value"

// Exists = empty string
var_dump($collection->get("two", "fallback"));
// ""

// Exists = null
var_dump($collection->get("three", "fallback"));
// null

// Does not exist
var_dump($collection->get("four", "fallback"));
// "fallback"

Proposed/old behavior

$data = [
    "one"   => "value",
    "two"   => "",
    "three" => null,
];

$collection = new Collection($data);

// Exists
var_dump($collection->get("one", "fallback"));
// "one"

// Exists = empty string
var_dump($collection->get("two", "fallback"));
// "fallback"

// Exists = null
var_dump($collection->get("three", "fallback"));
// "fallback"

// Does not exist
var_dump($collection->get("four", "fallback"));
// "fallback"

And your assertion is that we should keep the second option using empty() vs. isset()

Did I get this right?

[wurst-hans]

No, because v3 uses isset(), the old behavior should be (see the difference on second var_dump())

$data = [
    "one"   => "value",
    "two"   => "",
    "three" => null,
];

$collection = new Collection($data);

// Exists
var_dump($collection->get("one", "fallback"));
// "value"

// Exists = empty string
var_dump($collection->get("two", "fallback"));
// ""

// Exists = null
var_dump($collection->get("three", "fallback"));
// "fallback"

// Does not exist
var_dump($collection->get("four", "fallback"));
// "fallback"

So, IMO, the old behavior is better than the current:

1. When a key is set, even when it's an integer 0, a string "0" or an empty string, this value should be returned. This is the second use case
2. But even when a key exists but is NULL or a key does not exist (in both cases isset($this->lowerKeys[$element]) returns false), then ->get() should return the defined defaultValue. This are use cases 3 & 4.

I don't remember correctly, why we are talking about empty() and isset(). But to achieve the above behavior we should not use empty() or array_key_exists() but only isset(). Also see here

[niden]

Resolved in #15811