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

Allow a model to be both a parent and child #104

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

Conversation

chadhutchins
Copy link

The only reason this didn't work previously was due to duplicate method names in HasParent and HasChildren traits.

We have a use case where we have several layers of abstraction:

A extends B
B extends C

With this change we can now:

A HasParent
B HasParent, Has Children
C Has Children

and by C::find('1234') will return the class instance of A correctly.

…work previously was due to duplicate method names in HasParent and HasChildren traits.
…et the correct Parent Class name. If this does not happen, the wrong foreign keys are provided when pulling related data.
@tobias-grasse
Copy link

@driftingly Is there something to be done before this can be merged? I'd be interested in this functionality, so I'd be glad to help.

@driftingly
Copy link
Member

@tobias-grasse If you're able to, tests would be helpful.

@driftingly
Copy link
Member

I ran this update through the test suite of a larger project and didn't run into any issues.

Comment on lines 128 to +135
static $parentClassName;

return $parentClassName ?: $parentClassName = (new ReflectionClass($this))->getParentClass()->getName();
if ($parentClassName) return $parentClassName;

$parentClassName = (new ReflectionClass($this))->getParentClass()->getName();

$parent = new $parentClassName;
return $parent->hasParent === true ? $parent->getParentClass() : $parentClassName;
Copy link
Member

Choose a reason for hiding this comment

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

This may not work as expected.

Initially $parentClassName was set as static to cache the value. If $parentClassName had a value, it was returned; otherwise, a value was generated, assigned, then returned. The next time getParentClass() was called, the cached value was returned.

The updated code returns $parentClassName if a value exists.
If a value does not exist, one is generated and assigned.
Then we new up the parent class, and check if the parent class has a parent, if it does we call getParentClass() and return the result. The next time this method is called we will return the cached $parentClassName and not the result of $parent->getParentClass().

Does that make sense?

@chadhutchins
Copy link
Author

@tobias-grasse @driftingly this need is coming back up for me... either of yall interested in help me get this to the finish line? thanks!

@driftingly
Copy link
Member

@chadhutchins I haven't familiarized myself with the problem again, but I wonder if there's a way we can add another static variable to account for the two cases.

Like adding a static version of ParentParentClass, or maybe we only need to set $parentClass to the result of the parent parent class name, and it will be used next time.

We can also use reflection to see if the parent class has the HasParent trait instead of newing it up and checking.

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