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

Replaces __call with static methods #120

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

Conversation

shaoshiva
Copy link
Contributor

@shaoshiva shaoshiva commented Oct 2, 2017

Hi,

I don't see any reason to implement the search and suggest methods in __call instead of just exposing static methods ? Maybe I missed something ?

The problem of overloading the __call method from a trait is that other traits may overload it too, and it can't simply be solved by aliasing them because they all call parent::__ call, the only solution would be to manually merge the code of all the __call into a single method, but that's weird.

For example, I encountered this problem while using the Searchable trait in combination with this one :
https://github.com/Laravel-Backpack/CRUD/blob/master/src/ModelTraits/SpatieTranslatable/HasTranslations.php

@sleimanx2
Copy link
Owner

We won't be able to do the following.

$song = new Song();
$song->search()->...;

@shaoshiva
Copy link
Contributor Author

Oops sorry for the mess with the 2 last commits...

@jfadich
Copy link

jfadich commented Jul 13, 2018

Why would you realistically want to call search on a model instance? If you already have a model, then you wouldn't be searching.

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