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

Use protected so EntityGenerator can be extended #466

Merged
merged 1 commit into from
Nov 12, 2012
Merged

Use protected so EntityGenerator can be extended #466

merged 1 commit into from
Nov 12, 2012

Conversation

ethanresnick
Copy link
Contributor

This is definitely something I'd like to be able to extend, and I imagine others might too.

This is definitely something I'd like to be able to extend, and I imagine others might too.
@doctrinebot
Copy link

Hello,

thank you for positing this Pull Request. I have automatically opened an issue on our Jira Bug Tracker for you with the details of this Pull-Request. See the Link:

http://doctrine-project.org/jira/browse/DDC-2062

@zczapran
Copy link

zczapran commented Oct 9, 2012

Wouldn't it be better to create an interface and make the EntityGenerator implement it?

@ethanresnick
Copy link
Contributor Author

I don't think so. I mean, we could have an interface too, but the issue isn't so much that all EntityGenerator's needs to present the same methods to some other cooperating class.

The problem is that if I want to make tweaks to how generation works, I have to reimplement (read: copy and paste) the existing functionality into a new class and then make my tweaks there, rather than just subclassing the existing class.

@stof
Copy link
Member

stof commented Oct 9, 2012

@beberlei it may be time to complete your CodeGenerator project and migrate the EntityGenerator to it :)

@zczapran
Copy link

zczapran commented Oct 9, 2012

Some of the private (protected) methods (especially the ones that are complicated, like generateAssociationMappingPropertyDocBlock) could be moved into separate components and the EntityGenerator could be parametrized by injecting those components (composition over inheritance principle). It's a big refactoring work to be done but it might be worth it.

Anyway in a short term your approach is fine.

@ethanresnick
Copy link
Contributor Author

@stof @beberlei Hadn't seen that project before. Looks awesome.

@stof
Copy link
Member

stof commented Oct 9, 2012

@ethanresnick the goal is to rewrite the EntityGenerator entirely based on it.

beberlei added a commit that referenced this pull request Nov 12, 2012
Use `protected` so EntityGenerator can be extended
@beberlei beberlei merged commit 4e04daa into doctrine:master Nov 12, 2012
@beberlei
Copy link
Member

I will merge this as I have no defined roadmap when i will finish the code generator replacement.

@ethanresnick
Copy link
Contributor Author

Sounds good. Thanks!

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.

5 participants