Fixed plural variable names to singular when generating add or remove methods for entities #568

Closed
wants to merge 1 commit into
from

Projects

None yet

6 participants

@alexcarol

Changed generateEntityStubMethod so that variable names in add or remove methods are singular too

Edited tests for EntityGenerator so that variable names are checked too

@doctrinebot
Collaborator

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-2275

@Ocramius Ocramius commented on an outdated diff Feb 4, 2013
lib/Doctrine/ORM/Tools/EntityGenerator.php
@@ -1092,8 +1092,10 @@ protected function generateEntityFieldMappingProperties(ClassMetadataInfo $metad
protected function generateEntityStubMethod(ClassMetadataInfo $metadata, $type, $fieldName, $typeHint = null, $defaultValue = null)
{
$methodName = $type . Inflector::classify($fieldName);
- if (in_array($type, array("add", "remove")) && substr($methodName, -1) == "s") {
+ $variableName = Inflector::camelize($fieldName);
@Ocramius
Ocramius Feb 4, 2013 Doctrine member

Missing empty newline after this line. Also, align = signs

@Ocramius Ocramius commented on an outdated diff Feb 4, 2013
lib/Doctrine/ORM/Tools/EntityGenerator.php
$methodName = substr($methodName, 0, -1);
+ $variableName = substr($variableName, 0, -1);
@Ocramius
Ocramius Feb 4, 2013 Doctrine member

Align = signs

@alexcarol

Fixed coding standards

@beberlei
Member

This breaks backwards compatbilitiy, i am not sure if we can add this, even if semantically this would be correct.

@guilhermeblanco any idea?

@stof
Member
stof commented Mar 20, 2013

@beberlei I don't think this breaks BC as the EntityGenerator does not touch existing methods so changing the name of the local variable won't break things (PHP does not have named arguments so changing the argument name is not an issue)

@alexcarol

I agree with @stof, i don't really see how this breaks BC. Any pointers on that? If you want i can squash the commits and get this ready for the merge

@beberlei
Member
beberlei commented May 9, 2013

@stof @alexcarol The problem is when people regenerate their code, then the method names will change from those that existed before. Its true that methods are not changed, but sometimes people recreate entities completly new (EntityGenerator has an option for that).

@alexcarol

@beberlei but this change doesn't change the method name, it just changes the variable name (the method name was already being "singularized"

@bes89
bes89 commented Jul 14, 2013

@alexcarol Your changes will work but you have not fixed the problem completely. The <description>-placeholder needs to be replaced with the "singularized" variable name too.

I've created a gist:
https://gist.github.com/bes89/5995093

please compare it with your commits and fix it.

@alexcarol

Thanks for the input, will update asap

@alexcarol alexcarol Changed generateEntityStubMethod so that variable names in add and re…
…move methods and in the description are singular too.

Edited tests for EntityGenerator so that variable names are checked too
7e35e23
@alexcarol

ping @bes89 updated the PR, any other suggestions?

@bes89
bes89 commented Jul 23, 2013

No :-)
Am 22.07.2013 14:49 schrieb "Alex Carol" notifications@github.com:

ping @bes89 https://github.com/bes89 updated the PR, any other
suggestions?


Reply to this email directly or view it on GitHubhttps://github.com/doctrine/doctrine2/pull/568#issuecomment-21341511
.

@alexcarol

I guess there's no chance of this being merged, is there?

@beberlei
Member
beberlei commented Jan 2, 2014

@alexcarol no, only when you extract the current code into a method and then overwrite the entity generator in your code to fix this for you.

@beberlei beberlei closed this Jan 2, 2014
@beberlei
Member
beberlei commented Jan 2, 2014

Sorry to keep you hanging so long :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment