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

Don't remove setApplication suggestion. #788

Open
kromacie opened this issue Dec 22, 2020 · 1 comment
Open

Don't remove setApplication suggestion. #788

kromacie opened this issue Dec 22, 2020 · 1 comment

Comments

@kromacie
Copy link

kromacie commented Dec 22, 2020

Hi, in regard to the comment in Remote/Model

    /**
     * This should be compulsory in the constructor in the future,
     * but will have to be like this for BC until the next major version.
     *
     * @param Application $application
     *
     * @return $this
     */
    public function setApplication(Application $application)
    {
        $this->_application = $application;

        return $this;
    }

You've suggested that this is BC in the next major version but I think, it's really not necessary thing and may become annoying, as long as you are not always in the possession of credentials, for example when you have cached Invoice payload locally and only want to manipulate it, change, and save locally again. Or when you want to compare incoming payload with that locally cached one - that's kind of situation when you don't want to be able to save that cached models by accident. Application is used only when model is saved (for example in Repository if u follow that kind of pattern). But when it's not, its redundant dependency. What I'd suggest is to remove completely application context from Remote/Model and use Application->save/savaAll instead as it is now.

I'm open for discussion.

@kromacie
Copy link
Author

Furthermore, If you agree with that, I'd like to create pr to help you with this.

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

No branches or pull requests

1 participant