8000 Extend AbstractFactory for easy extending by skydiablo · Pull Request #11687 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Extend AbstractFactory for easy extending #11687

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Extend AbstractFactory for easy extending #11687

wants to merge 1 commit into from

Conversation

skydiablo
Copy link
Contributor

by this way, it is possible to use the hole abstract class and just override the 4 last getter functions. it will this class more reuseable!

@skydiablo
Extend AbstractFactory for easy extending
1618d5e 
by this way, it is possible to use the hole abstract class and just override the 4 last getter functions. it will this class more reuseable!
@Tobion
Copy link
Contributor
Tobion commented Aug 18, 2014

Use case?

@skydiablo
Copy link
Contributor Author

look here: https://github.com/Divi/AjaxLoginBundle/blob/master/DependencyInjection/Security/Factory/AjaxFormLoginFactory.php

if the service-id injected by a function, this class will be very handy. i'm in the same situation and have to complete reimplement the function "createAuthenticationSuccessHandler" and "createAuthenticationFailureHandler" only by changing the DefinitionDecorator-service-id.

@stof stof added the Security label Aug 18, 2014
@rybakit
Copy link
Contributor
rybakit commented Aug 25, 2014

Would it be more clear to add class constants and use them via static? The getDefaultAuthentication*HandlerServiceId() methods just return a string and I fail to see a use case where complex logic will be needed.

@fabpot
Copy link
Member
fabpot commented Sep 23, 2014

ref #11993

@fabpot
Copy link
Member
fabpot commented Sep 26, 2014

I think #11993 fixes your issue. Can you confirm?

@fabpot
Copy link
Member
fabpot commented Feb 11, 2015

Closing as there is no feedback and #11993 is probably a better alternative.

@fabpot fabpot closed this Feb 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Security
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@skydiablo @Tobion @rybakit @fabpot @stof
0