[DDC 559, DDC 852] Naming Strategy #241

Merged
merged 11 commits into from Dec 24, 2011

Conversation

Projects
None yet
6 participants
Owner

FabioBatSilva commented Dec 23, 2011

http://www.doctrine-project.org/jira/browse/DDC-559
http://www.doctrine-project.org/jira/browse/DDC-852

Hello,

This path add support for Naming Strategy,
That feature gives a handle to map naming convention of underlying tables and columns with the Objects and properties.

This is a big patch. So any suggestion are welcome :)

Thanks

Owner

guilhermeblanco commented Dec 24, 2011

@beberlei @asm89 @stof any comments?
I'm pretty happy with the patch and it seems very decent. =)

seems very useful as we mostly use userId and userProfile as names for columns and join tables.

@@ -21,7 +21,8 @@
use ReflectionClass, ReflectionProperty;
use Doctrine\Common\Persistence\Mapping\ClassMetadata AS IClassMetadata;
-
+use Doctrine\ORM\DefaultNamingStrategy;
+use Doctrine\ORM\NamingStrategy;
/**
@stof

stof Dec 24, 2011

Member

you should keep the empty line between the use statements and the phpdoc

lib/Doctrine/ORM/NamingStrategy.php
+ * <http://www.doctrine-project.org>.
+ */
+
+namespace Doctrine\ORM;
@stof

stof Dec 24, 2011

Member

these classes should probably be in the Mapping subnamespace

lib/Doctrine/ORM/NamingStrategy.php
+ *
+ * @param string $sourceEntity
+ * @param string $targetEntity
+ * @param string $propertyName
@stof

stof Dec 24, 2011

Member

missing @return

lib/Doctrine/ORM/NamingStrategy.php
+ *
+ * @param string $entityName
+ * @param string $referencedColumnName
+ */
@stof

stof Dec 24, 2011

Member

same here, and wrong indentation (missing one space)

lib/Doctrine/ORM/NamingStrategy.php
+ *
+ * @param string $propertyName A property
+ * @return string A column name
+ */
@stof

stof Dec 24, 2011

Member

wrong indentation

lib/Doctrine/ORM/NamingStrategy.php
+ */
+interface NamingStrategy
+{
+
@stof

stof Dec 24, 2011

Member

there is an extra line here

+ {
+ $string = preg_replace('/(?<=[a-z])([A-Z])/', '_$1', $string);
+
+ if ($this->case == self::CASE_UPPER) {
@stof

stof Dec 24, 2011

Member

you should use a strict comparison

@beberlei

beberlei Dec 24, 2011

Owner

Hm why self:: php already has constants for case. This is enough.

Member

stof commented Dec 24, 2011

@guilhermeblanco it seems a great idea, and a good way to implement it.

Owner

beberlei commented Dec 24, 2011

I like the patch, very good work.

+ */
+ private function underscore($string)
+ {
+ $string = preg_replace('/(?<=[a-z])([A-Z])/', '_$1', $string);
@beberlei

beberlei Dec 24, 2011

Owner

There is a method in common util inflector for this, please use that one

@FabioBatSilva

FabioBatSilva Dec 24, 2011

Owner

Hello @beberlei,
In the previous commit I try to use that, but i got a problem with UPPER_CASE_STRINGS
I got something like :

<?php
echo Inflector::tableize('UPPER_CASE_STRINGS');
// u_p_p_e_r__c_a_s_e__s_t_r_i_n_g_s```
@stof

stof Dec 24, 2011

Member

this seems like a bug in the Inflector IMO. @beberlei what do you think ?

@FabioBatSilva

FabioBatSilva Dec 24, 2011

Owner

This regex seems work fine,
If this is really a bug can I create one PR on common

Owner

FabioBatSilva commented Dec 24, 2011

Hello guys,

All done !

Please take a look. :)

Thanks for your comments.

+ *
+ * @param string $propertyName A property
+ * @return string A join column name
+ */
@stof

stof Dec 24, 2011

Member

the indentation of the phpdoc is still wrong here and for joinKeyColumnName

Owner

guilhermeblanco commented Dec 24, 2011

@stof you're a PHP lint!

I didn't see that it was out of Doctrine\ORM\Mapping namespace. Since @FabioBatSilva already moved, is there anything left? I'll be extremely happy to merge this one. =D

Member

stof commented Dec 24, 2011

@guilhermeblanco all the issue I saw have been fixed

guilhermeblanco added a commit that referenced this pull request Dec 24, 2011

Merge pull request #241 from FabioBatSilva/DDC-559
[DDC 559, DDC 852] Naming Strategy

@guilhermeblanco guilhermeblanco merged commit abb258c into doctrine:master Dec 24, 2011

Member

asm89 commented Dec 25, 2011

Nice PR! :)

@FabioBatSilva FabioBatSilva deleted the FabioBatSilva:DDC-559 branch Jan 18, 2013

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