Doctrine 2 - ORM

generate:entities creates bad method names with OneToMany

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 2.1
  • Fix Version/s: 2.1.1
  • Component/s: Tools
  • Security Level: All
  • Labels:
    None
  • Environment:
    Ubuntu 11.04, PHP 5.3.5, Symfony 2.0 RC4

Description

Hi. I noticed a minor bug in the generate:entities console task. I've got a Region entity that had the following property:

/**
 * @ORM\OneToMany(targetEntity="District", mappedBy="region")
 */
private $districts;

public function __construct()
{
    $this->districts = new ArrayCollection();
}

When I ran the generate:entities task it created the following methods:

/**
 * Add districts
 *
 * @param Fenix\StudyBundle\Entity\District $districts
 */
public function addDistricts(\Fenix\StudyBundle\Entity\District $districts)
{
    $this->districts[] = $districts;
}

/**
 * Get districts
 *
 * @return Doctrine\Common\Collections\Collection 
 */
public function getDistricts()
{
    return $this->districts;
}

The second method's name is correct, but what about addDistricts? Since it allows me to add a single District entity it should be as follows:

/**
 * Add district
 *
 * @param Fenix\StudyBundle\Entity\District $district
 */
public function addDistrict(\Fenix\StudyBundle\Entity\District $district)
{
    $this->districts[] = $district;
}

In other words, the term after "add" (as well as the parameter's name) should be the entity's name, not the property's one.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Benjamin Eberlei added a comment -

The problem is that making singular out of a word is not an easy task and i will not try to make this work somehow. The Entity Generator result is a helper, not necessarily generating the final result of a class. This is one case where the Code Generation will not nmake you happy, so just change it.

Show
Benjamin Eberlei added a comment - The problem is that making singular out of a word is not an easy task and i will not try to make this work somehow. The Entity Generator result is a helper, not necessarily generating the final result of a class. This is one case where the Code Generation will not nmake you happy, so just change it.
Benjamin Eberlei made changes -
Field Original Value New Value
Status Open [ 1 ] Resolved [ 5 ]
Resolution Invalid [ 6 ]
Hide
Permalink
Alessandro Desantis added a comment -

Couldn't you just use the target Entity's name (and not the property's one) in the "add" method?

Show
Alessandro Desantis added a comment - Couldn't you just use the target Entity's name (and not the property's one) in the "add" method?
Hide
Permalink
Benjamin Eberlei added a comment -

The shortname you mean? Yeah that would be an option you are right, i reopen the ticket

Show
Benjamin Eberlei added a comment - The shortname you mean? Yeah that would be an option you are right, i reopen the ticket
Benjamin Eberlei made changes -
Resolution Invalid [ 6 ]
Status Resolved [ 5 ] Reopened [ 4 ]
Hide
Permalink
Benjamin Eberlei added a comment -

Fixed.

This is sort of a BC break for 2.1, however since this is just a tool and generating code i think its acceptable

Show
Benjamin Eberlei added a comment - Fixed. This is sort of a BC break for 2.1, however since this is just a tool and generating code i think its acceptable
Benjamin Eberlei made changes -
Status Reopened [ 4 ] Resolved [ 5 ]
Fix Version/s 2.1.1 [ 10153 ]
Resolution Fixed [ 1 ]
Hide
Permalink
Alessandro Desantis added a comment -

Thanks a lot

Show
Alessandro Desantis added a comment - Thanks a lot
Hide
Permalink
Matthew Larson added a comment -

Another option for a semantic method name would be to use 'addTo' as the prefix, as in 'addToDistricts'.

The problem with using the target Entity's name (instead of the property's) is that sometimes you need to have multiple properties that map to the same target Entity.

Show
Matthew Larson added a comment - Another option for a semantic method name would be to use 'addTo' as the prefix, as in 'addToDistricts'. The problem with using the target Entity's name (instead of the property's) is that sometimes you need to have multiple properties that map to the same target Entity.
Benjamin Eberlei made changes -
Workflow jira [ 12812 ] jira-feedback [ 14945 ]
Benjamin Eberlei made changes -
Workflow jira-feedback [ 14945 ] jira-feedback2 [ 16809 ]
Benjamin Eberlei made changes -
Workflow jira-feedback2 [ 16809 ] jira-feedback3 [ 19062 ]
Christian Stoller made changes -
Comment [ In Doctrine 2.3 the 'add' and 'remove' methods in oneToMany associations have another problem (in earlier versions like 2.2 this worked correct). The singular form is not correctly detected if the property ends with 'ies' like 'entries' which should be transformed to 'entry'.
I have this YAML definition:

{code}
Archive:
  type: entity
  fields:
    id:
      id: true
      type: integer
      unsigned: false
      nullable: false
      generator:
        strategy: IDENTITY
  oneToMany:
    entries:
      targetEntity: Entry
      mappedBy: archive
{code}

This generates these methods:

{code}
public function addEntrie(\Entry $entries) { ... }
public function removeEntrie(\Entry $entries) { ... }
{code}

Because in the EntityGenerator only the plural 's' is removed. It would be nice if an ending of 'ies' could be replaced by 'y'. So that we get these methods

{code}
public function addEntry(\Entry $entries) { ... }
public function removeEntry(\Entry $entries) { ... }
{code}

My fork already has the changes https://github.com/naitsirch/doctrine-orm2/commit/a3adfccb4927d61da7debae46ed0fff61e4212f8
I have opened a pull request here https://github.com/doctrine/doctrine2/pull/530 ]
This list may be incomplete, as errors occurred whilst retrieving source from linked applications:
  • Request to http://www.doctrine-project.org/fisheye/ failed: Error in remote call to 'FishEye 0 (http://www.doctrine-project.org/fisheye/)' (http://www.doctrine-project.org/fisheye) [AbstractRestCommand{path='/rest-service-fe/search-v1/crossRepositoryQuery', params={query=DDC-1268, expand=changesets[-21:-1].revisions[0:29],reviews}, methodType=GET}] : Received status code 503 (Service Temporarily Unavailable)

People

  • Assignee:
    Benjamin Eberlei
    Reporter:
    Alessandro Desantis
Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: