Skip to content

[FrameworkBundle] Separate controller utils from its base class #20730

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

Closed
wants to merge 1 commit into from
Closed

[FrameworkBundle] Separate controller utils from its base class #20730

wants to merge 1 commit into from

Conversation

ro0NL
Copy link
Contributor

@ro0NL ro0NL commented Dec 3, 2016

Q A
Branch? "master"
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? not yet
Fixed tickets #16863
License MIT
Doc PR reference to the documentation PR, if any

My version of decoupling controller utilities from the controller base class.

It's a WIP as not all utils are transferred yet and tests are failing by the way in which mocks are built. (the annoying way).

Goals;

  • dont force a controller base class
  • dont inject the service container
  • dont inject the same services over and over again in each controller to avoid above
  • avoid traits to do the above
  • allow for reuseabality of those very useful utilities :))
  • clean and explicit dependencies

This could deprecate Controller::generateUrl in favor of ControllerUtils::generateUrl (along with others).

// before
$this->generateUrl();

// after
$this->get('controller_utils')->generateUrl();

Idea is to do the same thing for security related utils, so ControllerUtils can inject and forward to SecurityUtils.

@nicolas-grekas
Copy link
Member

👎 it creates overhead for DX, I don't see the benefit over #18193 or #16863

@dunglas
Copy link
Member

dunglas commented Dec 16, 2016

👎, I prefer the approach of using a trait too.

@ro0NL
Copy link
Contributor Author

ro0NL commented Dec 16, 2016

The benefit would be that utility methods can be reused in any context, not just when using the base controller class.

I can built this in user-land, no problem. But ideally the absolute definition of utility methods are shipped by SF.

However im still in doubt about the right approach here, as this could lead to people injecting the util class whereas only routing is needed for instance, in that case using a trait probably works out better.

Im closing it, as for now ill just extend the base controller which practically gives the same result.

@ro0NL ro0NL closed this Dec 16, 2016
@ro0NL ro0NL deleted the controller-utils branch December 16, 2016 10:27
@nicolas-grekas nicolas-grekas modified the milestones: 3.x, 3.3 Mar 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants