Improve Eloquent casting for collection type

arcanedev-maroc commented 7 years ago

I'm wondering if it's a good idea to return an empty Collection object instead of null when we specify a casting like this:

// Meta Model Class

protected $casts = [
    'keywords' => 'collection',
];

This will avoid us to add more checks like is_null($this->keywords). And it's views friendly when you do something like this {{ $meta->keywords->implode(', ') }}.

And for if statement, i prefer to do something like @if ($meta->keywords->isEmpty()) than @if (is_null($meta->keywords)).

So, my PR will be something like this (At this section on master branch):

/**
 * Cast an attribute to a native PHP type.
 *
 * @param  string  $key
 * @param  mixed  $value
 * @return mixed
 */
protected function castAttribute($key, $value)
{
    if (is_null($value)) {
        return $this->castNullAttribute($key, $value);
    }

    switch ($this->getCastType($key)) {
        case 'int':
        case 'integer':
            return (int) $value;
        case 'real':
        case 'float':
        case 'double':
            return (float) $value;
        case 'string':
            return (string) $value;
        case 'bool':
        case 'boolean':
            return (bool) $value;
        case 'object':
            return $this->fromJson($value, true);
        case 'array':
        case 'json':
            return $this->fromJson($value);
        case 'collection':
            return new BaseCollection($this->fromJson($value));
        case 'date':
            return $this->asDate($value);
        case 'datetime':
            return $this->asDateTime($value);
        case 'timestamp':
            return $this->asTimestamp($value);
        default:
            return $value;
    }
}

/**
 * Cast a null attribute.
 *
 * @param  string  $key
 * @param  mixed  $value
 * @return mixed
 */
protected function castNullAttribute($key, $value)
{
    switch ($this->getCastType($key)) {
        case 'collection':
            return new BaseCollection;
        default:
            return $value;
    }
}

We can also add array to the casts instead returning a null value.

/**
 * Cast a null attribute.
 *
 * @param  string  $key
 * @param  mixed  $value
 * @return mixed
 */
protected function castNullAttribute($key, $value)
{
    switch ($this->getCastType($key)) {
        case 'array':
            return [];
        case 'collection':
            return new BaseCollection;
        default:
            return $value;
    }
}

cc: @taylorotwell @themsaid @GrahamCampbell

sisve commented 7 years ago

This seems related to #9 (Improve Typecasting)

taylorotwell commented 7 years ago

I agree an empty collection may make sense in this situation. But haven't thought through it fully.

suadhuskic commented 6 years ago

+1

linaspasv commented 6 years ago

@arcanedev-maroc when could we expect PR for this?

sisve commented 6 years ago

It would make sense to add a not-null collection cast (another cast) for this feature to allow code to check for null values vs empty collections. Different models (or same model in different context) may behave differently.