8000 [6.x] Added creation of custom Cast classes for Eloquent by andrey-helldar · Pull Request #30953 · laravel/framework · GitHub
[go: up one dir, main page]
Skip to content

[6.x] Added creation of custom Cast classes for Eloquent #30953

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

[6.x] Added creation of custom Cast classes for Eloquent #30953

wants to merge 2 commits into from

Conversation

andrey-helldar
Copy link
Contributor

Recently, my #30931 PR was rejected and I understand the reason, since my proposal implies an increase in the amount of processed code. This is not entirely good.

In exchange, I suggest introducing a new type of object - custom Cast objects that allow developers to choose how to handle their values.

The application process is as follows:

  1. Using the php artisan make:cast command, the developer creates a cast file.
  2. Inside the created file (by default in the app/Http/Casts directory), the developer himself determines the behavior of the handle method:
use Illuminate\Contracts\Database\Eloquent\Castable;

class MyCast implements Castable
{
    public function handle($value = null)
    {
        return $value % 10;
    }
}
  1. In the model, you need to specify the created caste:
use App\Http\Casts\MyCast;

public $casts = [
    'user_field' => MyCast::class,
];
  1. Profit :)

In my opinion, this approach will not only save the amount of code for the “box” of the framework, but also significantly expand its functionality.

In addition, I removed the methods for working with castes from the hasAttributes trait, which allowed me to keep the code cleaner.

@taylorotwell
Copy link
Member

Other casts work both ways. Get and set. This is only "get"?

@andrey-helldar
Copy link
Contributor Author

@taylorotwell, yes, there was a mistake.
I’ll figure out how to solve this problem and send PR again 😊

@mfn
Copy link
Contributor
mfn commented Dec 27, 2019 •
edited
Loading

I'm pretty sure there was a previous attempt. I already wanted to bring it up with your original PR; don't need to do now I guess.

What I want to say: looking for such a feature for some time, seems really super useful. Looking forward.

Upfront:

  • if the changes are as big as in this PR, you can already target master I believe because these are likely going to be breaking changes (subclassing)
  • benchmarks would be nice, just "to be sure"

@andrey-helldar
Copy link
Contributor Author

@mfn, this code does not break existing functionality, but supplements it with a new feature. So I posted the changes to the sixth branch.

@andrey-helldar
Copy link
Contributor Author
andrey-helldar commented Dec 27, 2019 •
edited
Loading

@mfn, see new #30958 PR
I will fix the tests in the new PR as soon as I am at the computer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@andrey-helldar @taylorotwell @mfn
0