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

Entity refactor #2002

Merged
merged 4 commits into from
May 13, 2019
Merged

Entity refactor #2002

merged 4 commits into from
May 13, 2019

Conversation

lonnieezell
Copy link
Member

Refactoring Entity to:

  • provide more accurate changed value determiniation
  • allow null values easier
  • improve usability of Entities in general
  • New methods provided to syncOriginal() values based on the saved values.

With these changes, the _options array is no more, and the datamap, casts, and dates are now class vars of their own. Two new properties exist on Entities: original and attributes to store the current and original state of values. You no longer have to set property values for each column in the database.

@lonnieezell lonnieezell self-assigned this May 13, 2019
@lonnieezell lonnieezell changed the title WIP Entity refactor Entity refactor May 13, 2019
@lonnieezell lonnieezell merged commit 4699642 into develop May 13, 2019
@chistel
Copy link

chistel commented May 13, 2019

Nice, sharing a quick thought, when save data, the model should be able to return the entity class without me me having to use

$user = new App\Entities\User($data); when saving and then call the model itself when updating. It seems to be more of a waste.

@MGatner
Copy link
Member

MGatner commented May 13, 2019

Yeah this looks great. I'm curious if you considered hasChanged() without parameters, as an option to check if any properties have been changed?

@lonnieezell
Copy link
Member Author

@MGatner I didn't think of that, but that addition would make sense.

@lonnieezell
Copy link
Member Author

@chistel That's a good point. Will have to look into that, unless someone wants to beat me to it :)

Should look into whether that makes sense for any other data type, also....

@MGatner
Copy link
Member

MGatner commented May 13, 2019

@lonnieezell For a whole-object hasChanged() are you comfortable with simply:

return $this->original !== $this->attributes

i.e. letting array comparison do the heavy lifting?

@lonnieezell
Copy link
Member Author

@MGatner My initial instinct was that we should do a loose comparison, but I think doing it that way is fine. Even if they set what was originally an int 1 with a string "1" it's still technically changed, though it wouldn't have any affect. For the use cases I imagine this working in a strict comparison should be cool.

@chistel
Copy link

chistel commented May 13, 2019

OK @lonnieezell if I do have free time, I'll look into it

@tada5hi
Copy link
Contributor

tada5hi commented May 13, 2019

i think in the Constructer ( __construct() ) the syncOriginal() method call should be after fill($data), because to that time the data hold in the attribute of the class is the original data, and in the current order the syncOriginal() just syncs an empty array.

@lonnieezell
Copy link
Member Author

If sync is done after full - then save will never work as expected because it will think nothing has changed. The sync before is only there in case people choose to set default values in their class.

@lonnieezell lonnieezell deleted the entityrefactor branch May 20, 2019 04:26
@sfadschm sfadschm mentioned this pull request Dec 1, 2020
1 task
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.

4 participants