Feature: add isSet() method to identify properties without a value

[stratease]

Avoid a false positive in fetching defaults with a check that validates whether one is truly set or not. This avoids situations where we might force a null into a database, instead of allowing the MySQL default to take precedence.

[JasonTheAdams]

Hi @stratease!

I like this. I want to think through it a bit. The isset() function considers null to be a non-value. So I believe the following reveals the value isn't set:

class Foo extends Model
{
  protected $properties = [
    "bar" => "int"
  ];
}

$foo = new Foo();
isset($foo->bar); // false

Now it's not possible to know from this method whether a value started as null or was made null after the fact. For example, a model could be retrieved from the database, a property made null, and then the model saved. Or it could be a new model with a property explicitly set as null.

It looks like you're using array_key_exists to circumvent this. I'm confused by the [1] present and the hard-coded 1 check. What's going on there?

[stratease]

Hi @stratease!

I like this. I want to think through it a bit. The isset() function considers null to be a non-value. So I believe the following reveals the value isn't set:

class Foo extends Model
{
  protected $properties = [
    "bar" => "int"
  ];
}

$foo = new Foo();
isset($foo->bar); // false

Now it's not possible to know from this method whether a value started as null or was made null after the fact. For example, a model could be retrieved from the database, a property made null, and then the model saved. Or it could be a new model with a property explicitly set as null.

Correct, I'm with you there. Lots of nuance around checking an explicitly set null versus one that hasn't been set.

It looks like you're using array_key_exists to circumvent this. I'm confused by the [1] present and the hard-coded 1 check. What's going on there?

Not sure the "why" but it appears we define a default with this shape 'firstName' => [ 'string', 'Michael' ] see our test Model. This is to determine we explicitly have a default defined or nothing.

Thoughts?

[JasonTheAdams]

Not sure the "why" but it appears we define a default with this shape 'firstName' => [ 'string', 'Michael' ] see our test Model. This is to determine we explicitly have a default defined or nothing.

Ohhh! Right, hahah! I was thinking about the wrong thing. That's just a visual syntax sort of thing.

My suggestion is to make hasDefault a protected function, and add an $model->isSet('foo') function that uses array_key_exists on $this->attributes[] to actually know the difference between whether a model is truly missing a property value.

[stratease]

Updated per comments.

[JasonTheAdams]

This is awesome! What a great addition, @stratease! 🙌

[JasonTheAdams]

@stratease I'm trying to think. Is this change backwards-breaking in any way? I don't think so. We're not setting a null value in attributes anymore for every property upon instantiation. But I don't think it actually changes the behavior of any public methods. 🤔

[stratease]

@stratease I'm trying to think. Is this change backwards-breaking in any way? I don't think so. We're not setting a null value in attributes anymore for every property upon instantiation. But I don't think it actually changes the behavior of any public methods. 🤔

@JasonTheAdams I don't think it will but there might be a possibility if we assume it will always fill undefined attributes with a null, now it won't. I think toArray() will only return what was explicitly set now, and not every property regardless of what was hydrated. Which was the intended result.