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

Update Model.php #870

Merged

Conversation

Healyhatman
Copy link
Contributor

Add a static constructor so can more cleanly chain after instantiation, like this

$rate = EarningsRate::make($application)->setEarningsType(...)->someOtherThing(...);

Remove the EARNINGTYPE_FIXED rate because it's definitely not valid in Xero anymore. If you don't think that's the right call let me know.

Add a static constructor so can more cleanly chain after instantiation

Remove the EARNINGTYPE_FIXED rate because it's definitely not valid in Xero anymore
@calcinai
Copy link
Owner

@Healyhatman are you doing this to match style with a particular framework? I'm just wondering about which verb is mose appropriate (I see ::create() fairly regularly. I assume you're just trying to avoid

$rate = (new EarningsRate($application))->setEarningsType(...)->someOtherThing(...)

Which I also see a bit.

@Healyhatman
Copy link
Contributor Author

It matches Laravel, where create() would immediately store a model in the database but make would just instantiate it

@Healyhatman
Copy link
Contributor Author

Healyhatman commented Sep 29, 2022

@calcinai yes just trying to avoid (new Model($app))-> because it makes me sad

@calcinai calcinai merged commit b132098 into calcinai:master Sep 30, 2022
@calcinai
Copy link
Owner

Agreed

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.

2 participants