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

Add support for casting to Decimal via cast table #3

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AJenbo
Copy link

@AJenbo AJenbo commented May 8, 2023

First of thanks for a nice extensions 👍

The idea here is to make it easy for developers to use the Decimal class directly on models rather then having to implement mutators for every attribute.

<?php
namespace App;

use Decimal\Decimal;
use Decimal\DecimalObjectCast;

/**
 * @property Decimal $value
 */
class Item extends Model
{
    use DecimalObjectCast;

    protected $casts = [
        'oldValue' => 'decimal:2', // Laravel format, will default to Decimal::DEFAULT_PRECISION
        'value' => 'decimal:2:8', // Extended format with precision
    ];
}

This also extends the format to allow for providing the precision value in the cast string

@rtheunissen
Copy link
Contributor

Thank you for this PR, I think this is a good idea.

  1. I don't like the name Decimal2, maybe we can use Cast\Decimal instead and make it more general. To do this, let's have the class generate a caster object, so you would have something like Decimal\Cast\Fixed(2) return an object that would cast to that number of decimal places.
  2. The use of the value's precision as the number of decimal places in your cast implementation is not quite right, because precision is the number of significant figures in the number overall. We should either have that number passed in, or somehow infer it from the database.

@AJenbo
Copy link
Author

AJenbo commented May 9, 2023

Thanks for explaining that.

I have adjusted things to instead use the trait as that turned out to be the most sensible way to implement the suggestions.
(cast classes cannot access the casts property and so do not have access to the scale and precision)

This has the benefit of the trait now handling casting both to and from the string representation

Mode:

<?php
namespace App;

use Decimal\Decimal;
use Decimal\DecimalObjectCast;

/**
 * @property Decimal $value
 */
class Item extends Model
{
    use DecimalObjectCast;

    protected $casts = [
        'oldValue' => 'decimal:2', // Laravel format, will default to Decimal::DEFAULT_PRECISION
        'value' => 'decimal:2:8', // Extended format with precision
    ];
}

@AJenbo AJenbo marked this pull request as draft May 9, 2023 01:44
@AJenbo AJenbo marked this pull request as ready for review May 9, 2023 02:28
@AJenbo AJenbo changed the title Create convinient cast class for numbers with 2 decimal positions Add support for casting to Decimal via cast table May 9, 2023
@AJenbo
Copy link
Author

AJenbo commented May 9, 2023

I made the format decimal:decimals:precision rather then the MySQL form of DECIMAL(precision, decimals) so to stay compatible with Laravels decimal:decimals format.

@AJenbo
Copy link
Author

AJenbo commented May 14, 2023

After tresting things a bit I found that it could actually be done a bit simpler. Now the only overwrites are for castAttribute() and setAttribute(). The logic is made to mirror that of the recent addition for enums in Laravel so should be pretty clean and by the book.

@rtheunissen any feedback?

@rtheunissen
Copy link
Contributor

I've been away on vacation but I've been thinking about this work and I think overall it is a good idea that I'll take a closer look at this week and merge.

@jameshulse
Copy link

jameshulse commented Jul 5, 2023

As of Laravel 7, the recommended way to perform custom casting is via a Cast class (https://laravel.com/docs/10.x/eloquent-mutators#custom-casts). You can still pass parameters to these, but it would likely be slightly 'safer' than using a trait to override the cast method.

Example usage:

<?php
namespace App;

use Decimal\Decimal;
use Decimal\DecimalObjectCast;

/**
 * @property Decimal $value
 */
class Item extends Model
{
    use DecimalObjectCast;

    protected $casts = [
        'oldValue' => DecimalObjectCast::class.':2', // Laravel format, will default to Decimal::DEFAULT_PRECISION
        'value' => DecimalObjectCast::class.':2:8', // Extended format with precision
    ];
}

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.

3 participants