Adding support for nullable/optional fields?

[BrekiTomasson]

First of all; I love the package, it replaced a number of helper methods that I had written in my own codebase to solve similar problems when working with data from external APIs etc.

One problem I'm having with it, however, is that some of the APIs I'm working with return "" (empty strings) for fields that have no data. I can't ->getAsInt() these fields, since "" can't be cast to an integer. Would it be possible to add support for something like this:

>>> $field = "";
>>> $mess = new Mess($field);
>>> $mess->getAsNullableInt();
=> null

I'll leave the naming to you, of course, but ->getAsNullableInt() or ->getAsIntOrNull() seem reasonable to me.

[BrekiTomasson]

Looking through the codebase now, I can see that you've got functions like toInt($value) and toFloat($value) that return null if the value cannot be cast to that type, so maybe this is what you're supposed to use instead of instantiating a new Mess and using the built-in methods?

[zakirullin]

@BrekiTomasson Hi there! Thanks for your feedback!

Originally, findAsInt() method is supposed to act like "find me something that looks like an integer or return null". We came up with strict and naive casting rules to forget about all those complicated PHP's castings like (int)"123abc" => 123 and so. "" is definitely not something that looks like an int.

While at the same time I admit, that in real world there are lots of poor designed API, which aren't enforced by any strict contracts. Those APIs don't differentiate between null, "" or missing key (or sometimes even between 0). At that is freaking annoying. All their clients are obliged to invent some weird casting logics, that tends to fall apart once in a while.

With that being said, we have to deal with all that messy stuff somehow. But does that fall into this library's scope? That question is yet to be answered. I strongly believe that we'll end up with a set of complicated rules akin to PHP's current implementation, thus loosing casting clarity and simplicity. I.e. at the point we change the definition of null, by saying that "" = null, we're crossing that sloppy line between type casting and business validation. At the later is definitely out of current library's scope.

P.S. Recently we've spend some time discussing exactly this problem with @remorhaz, maybe he has something to add up here

[BrekiTomasson]

You're absolutely right, of course. It's far better for this library to remain "strict" when it comes to types and handle business logic like detecting "empty" fields separately.

That said, it does get quite ugly when I hve to do something like:

• Fetch JSON data from an external API into a Laravel Collection.
• Iterate over all fields and convert any "empty" values ("" and "0", for example) to null and use Mess for those that are not empty.

As it stands, I've created two helper methods that wrap around Mess, so that I can write code using Mess more fluently:

function mess(mixed $value) : Mess
{
    return new Mess($value);
}

function nullable_mess(mixed $value) : Mess|null
{
    if (empty($value)) {
        return null;
    }

    return new Mess($value);
}

Doing it this way, and using nullsafe operations, I can write logic like nullable_mess($response[0]['country_id'])?->getAsInt() - but it still feels like a somewhat ugly workaround. ;)

[remorhaz]

Hi, @BrekiTomasson !

In my case I had been solving a problem of providing typed access to query parameters. In my case, query ?foo=&bar=1 should interpret foo parameter as null, but the problem was exactly the same as yours. In case of scalar we can cast empty string to null before wrapping it with accessor, but that will not work for values within structures.

So, after having a bit of thinking about the problem and talking to @zakirullin , I think that we should introduce additional abstraction such as value provider. It should implement ArrayAccess and provide two additional methods: get(): mixed (to get current value) and exists(): bool (to check for existance). And Mess should work with this abstraction inside. This will allow us to virtualize data, not limiting them to native types.

[BrekiTomasson]

Yeah, the problem you were having is very similar to mine, and I'm guessing the solution (well, more like 'fairly ugly workaround') that I came up with is likely something that'll work on your end too. Just out of curiosity, how did you solve it on your end? Did you do a "nullable" check first and - if not nullable - then use Mess, or did you do it my way by wrapping Mess inside of a helper function that takes care of it?

I really like the idea of the additional abstraction of value providers, by the way. Eager to keep an eye on the repo to see how this turns out. Lemme know if you want any help or inspiration along the way, I'd be happy to throw a PR or two your way.

[remorhaz]

In my case, I had to made a "quick-n-dirty" solution:

1. I wrote custom MessInterface implementation.
2. It's constructor accepted two MessInterface instances: for original value ($nestedAccessor) and for default one ($defaultAccessor).
3. I wrote private method getParamAccessor(): MessInterface (see below).
4. I made all get/find methods return result of the same method of getParamAccessor() result.
5. I made getOffset() interface to return new self($this->nestedAcessor[$offset], $this->defaultAccessor[$offset]).

    private function getParamAccessor(): TypedAccessorInterface
    {
        try {
            /** @psalm-var mixed $value */
            $value = $this->nestedAccessor->getMixed();

            return '' === $value || null === $value
                ? $this->defaultAccessor
                : $this->nestedAccessor;
        } catch (MissingKeyException) {
            return $this->defaultAccessor;
        }
    }

So, the idea is the following: just switch between two accessors depending on value's "nullishness", and let the native implementation do the rest.

[zakirullin]

On the other had, if we're dealing with messy stuff here anyway, maybe some common solution would do.

Something like (kinda v2 api):

int(): int // getInt
nullableInt(): ?int // findInt
emptyableInt(): ?int // null (for null and '') or int

asInt()
asNullableInt()
asEmptyableInt()

nullableArrayOfStringToInt()
nullableList()
...
etc.

What do you think?

Yes, it has no extension point, compared to @remorhaz 's value provider solution, but maybe we don't need it here for the sake of simplicity and dumbness.

[zakirullin]

It's just I am a bit concerned with the cognitive load one should bear once we introduce some additional value provider abstraction.

I.e. once a reader sees getInt() call, he would have to travel all the way up to the Mess instantiation to see what kind of ValueProvider was provided. Then one should keep that insight in his short time memory, like "Aha, this instance of Mess converts empty strings to nulls". We kinda become stateful.

[remorhaz]

I am a bit concerned with the cognitive load one should bear once we introduce some additional value provider abstraction

Most users will never deal with it. It's only for the cases when you need to override "value", "path" and "existance".

maybe we don't need it here for the sake of simplicity and dumbness

It's not a client of MessInterface who should decide to do workarounds or not. Client just needs a string, it should know nothing about the original nature of the "value" and the criteria of its "emptiness". The abstraction will leak if we do so.