Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.4] Add Caster Classes #16460

Closed
wants to merge 1 commit into from
Closed

[5.4] Add Caster Classes #16460

wants to merge 1 commit into from

Conversation

faustbrian
Copy link
Contributor

This makes use of classes for casting models instead of using a switch statement for each single type to perform some action on the value.

<?php

class Invoice extends Eloquent
{
    protected $casts = [
        'amount_due' => 'money'
    ];

    protected $customCasters = [
        'money' => \App\Casters\MoneyCaster::class
    ];
}
<?php

class MoneyCaster extends AbstractCaster
{
    public function as($value)
    {
        return $value;
    }

    public function from($value)
    {
        return Money((int) $value);
    }
}

Copy link
Contributor

@DuckThom DuckThom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as Is a reserved keyword in php and cannot be used as function name.

http://php.net/manual/en/reserved.keywords.php

@taylorotwell
Copy link
Member

This is essentially similar to the original model cast PR I already submitted. We may re-open that one if interest continues.

@CristianLlanos
Copy link

CristianLlanos commented Nov 18, 2016

@taylorotwell I believe this approach is a bit different to your proposal #13706. In your proposal we can only cast attributes into objects whereas with this alternative we can use it to transform attributes to whatever value we want. For example, decoding from base64 string #16450. It will be up to the developer to take care of value objects if that's what they need/want; but this PR is not about casting attributes into objects, it's more generic.

@CristianLlanos
Copy link

@JosephSilber, I think this gives us the freedom we needed.

@faustbrian
Copy link
Contributor Author

faustbrian commented Nov 18, 2016

@taylorotwell As @CristianLlanos pointed out this implementation is kept quite generic and opposed to your PR this leaves the developer free hand as to what he wants to do with the value passed to the caster.

I guess you could describe this implementation more as a generic transformer for model attributes.

@faustbrian
Copy link
Contributor Author

faustbrian commented Nov 26, 2016

@CristianLlanos Moved the PR to a package https://github.com/faustbrian/Eloquent-Castable. Maybe this will find it's way into Laravel one day.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants