[DDC-2316] [GH-588] ClassMetadataInfo: use reflection for creating new instance (on PHP >=5.4) Created: 23/Feb/13  Updated: 04/May/13

Status: Open
Project: Doctrine 2 - ORM
Component/s: None
Affects Version/s: None
Fix Version/s: 3.0
Security Level: All

Type: Improvement Priority: Major
Reporter: Benjamin Eberlei Assignee: Benjamin Eberlei
Resolution: Unresolved Votes: 0
Labels: None


 Description   

This issue is created automatically through a Github pull request on behalf of Majkl578:

Url: https://github.com/doctrine/doctrine2/pull/588

Message:

On PHP >=5.4, use proper way for instantiating classes without invoking constructor.



 Comments   
Comment by Benjamin Eberlei [ 04/May/13 ]

Scheduling this for 3.0, when we move to php 5.4 or higher requirement





[DDC-2133] Issue with Query::iterate and query mixed results Created: 09/Nov/12  Updated: 01/May/13

Status: Open
Project: Doctrine 2 - ORM
Component/s: ORM
Affects Version/s: 2.2.1
Fix Version/s: 3.0
Security Level: All

Type: Bug Priority: Major
Reporter: Oleg Namaka Assignee: Benjamin Eberlei
Resolution: Unresolved Votes: 0
Labels: None

Issue Links:
Duplicate
is duplicated by DDC-1314 DQL permits partial select using SQL Resolved

 Description   

Consider this code:

$dql = "
    SELECT Page, Product.name
    FROM Dlayer\\Entity\\Page Page
    INNER JOIN Page.Product Product
    ";
$q = ($em->createQuery($dql));
foreach ($q->iterate() as $entry) {
  $page = $entry[0][0];
  $name = $entry[0]['name'];
}

This results with undefined index: 'name' for the second entry.

First result keys are (notice just one array element with index 0):

0
array(2) {
  [0] =>
  int(0)
  [1] =>
  string(4) "name"
} 

but all others are different (notice two array elements with index 0 and the other one that is incrementing):

the second one:
0
array(1) {
  [0] =>
  int(0)
}
1
array(1) {
  [0] =>
  string(4) "name"
} 
the third one:
0
array(1) {
  [0] =>
  int(0)
}
2
array(1) {
  [0] =>
  string(4) "name"
} 

What's wrong with this approach? Is it a bug or mixed results should not be used with the iterate method?



 Comments   
Comment by Benjamin Eberlei [ 12/Nov/12 ]

This is a known issue that we don't have found a BC fix for and as I understand Guilherme Blanco requires considerable refactoring.





[DDC-2089] Modify OneToMany to allow unidirectional associations without the need of a JoinTable Created: 19/Oct/12  Updated: 07/Sep/13

Status: Open
Project: Doctrine 2 - ORM
Component/s: ORM
Affects Version/s: 2.x
Fix Version/s: 3.0, 2.5
Security Level: All

Type: Improvement Priority: Major
Reporter: Enea Bette Assignee: Benjamin Eberlei
Resolution: Unresolved Votes: 0
Labels: onetomany, persister, unidirectional
Environment:

Debian Wheezy, Mysql 5.1, Apache2, PHP 5.4



 Description   

As I sayd in the title, it would be nice if the ORM layer could permit to map a 1:n association in the db as an unidirectional OneToMany in the classes, without using a JoinTable in the database.
This would permit us to get rid of the unnecessary database JoinTable, which creates disorder and decreases performance for no valuable reason.

Is it possible?



 Comments   
Comment by Enea Bette [ 16/Dec/12 ]

A little up... for inspiration from JPA

http://en.wikibooks.org/wiki/Java_Persistence/OneToMany#Undirectional_OneToMany.2C_No_Inverse_ManyToOne.2C_No_Join_Table_.28JPA_2.0_ONLY.29





[DDC-536] Remove the _ prefix from private and protected members Created: 23/Apr/10  Updated: 16/Apr/14

Status: Open
Project: Doctrine 2 - ORM
Component/s: None
Affects Version/s: None
Fix Version/s: 3.0
Security Level: All

Type: Improvement Priority: Major
Reporter: Roman S. Borschel Assignee: Guilherme Blanco
Resolution: Unresolved Votes: 1
Labels: None


 Description   

The reasoning is simple: The prefix "_" is usually either used for easier distinction of instance variables from other, i.e. local variables, instead of always using "this." (often seen in C#), or it is used to signal that a member is not meant to be accessed from outside of the class when the language does not have visibility modifiers (PHP4).

Since you always have to use "$this->" in PHP5+ when accessing instance members and there are visibility modifiers, the "_" is largely superfluous and just makes the verbose OO code even more verbose.

Maybe the following find/replace steps will do the job almost completely:

"private $_" => "private $"
"protected $_" => "protected $"
"$this->_" => "$this->"


 Comments   
Comment by Benjamin Eberlei [ 27/Apr/10 ]

i just found a possible BC issue with this.

EntityRepository is allowed to be extended by us, it has several variables that are underscore prefixed. How to proceed in this case?

Comment by Roman S. Borschel [ 27/Apr/10 ]

I know but its not really a problem I think. We should just decide whether we make them private in the first place and provide getters instead (which would have avoided this problem in the first place).

Comment by Roman S. Borschel [ 27/Apr/10 ]

Leaving the prefixes on the repository class only is also an option... but I dont think thats necessary.

Comment by Benjamin Eberlei [ 27/Apr/10 ]

can we commit getters for Beta 1 then? We could give everyone a period until Beta 2 to fix their code and then make the change.

EntityRepository is the only class that is meant to be userland extendable to my knowledge, so this should be the only problem to adress

Comment by Roman S. Borschel [ 27/Apr/10 ]

Yes, you can add getters and commit right away if you want. Plus adding a note on the upgrade document that direct access of these properties is deprecated.

Comment by Roman S. Borschel [ 27/Apr/10 ]

Persisters will be also extensible some day in userland but they need more polish for that, I've already started with it

Comment by Johnny Peck [ 19/Nov/10 ]

Is this still planned? Searching the code base finds this is not being implemented. It would be a good idea to implement the change sooner than later if it will be done at all. Also, +1 for the change. It makes complete sense.

Comment by Guilherme Blanco [ 16/Apr/14 ]

Moving to 3.0 as this can potentially create BC breaks





[DDC-274] Class and namespace naming inconsistency Created: 24/Jan/10  Updated: 16/Apr/14

Status: In Progress
Project: Doctrine 2 - ORM
Component/s: None
Affects Version/s: None
Fix Version/s: 3.0
Security Level: All

Type: Improvement Priority: Critical
Reporter: Glen Ainscow Assignee: Guilherme Blanco
Resolution: Unresolved Votes: 1
Labels: None


 Description   

There are inconsistencies with some class and namespace names that include acronyms.

Examples:

Classes with upper-casing:
ORMException, DBALException, OCI8Connection, etc.

Classes with proper-casing:
RunDqlTask, CliException, MySqlPlatform, etc.

Namespaces with upper-casing:
DBAL, ORM, Doctrine\DBAL\Driver\PDOMsSql, etc.

Namespaces with proper-casing:
Doctrine\Common\Cli, Doctrine\DBAL\Tools\Cli\, Doctrine\ORM\Id, etc.

There is more proper-casing than upper-casing. IMHO, proper-casing is better as it's easier to read "SqlException" than it is to read "SQLException" (the "E" looks like part of the acronym), and things like "CLITask" can be avoided.

I discussed this a bit with Benjamin and Guilherme, and they were unsure and said that the whole team needed to reach consensus.

I'm leaving the priority as "Major" because this should probably be fixed sooner rather than later to prevent compatibility breaks.



 Comments   
Comment by Guilherme Blanco [ 25/Jan/10 ]

Increasing the severity and adding a fix version since this MUST be fixed before next release.

Comment by Roman S. Borschel [ 25/Jan/10 ]

I find this to be of rather minor importance. You're talking of compatibility breaks, but thats only the case if we intend to start being very nitpicky about the naming in the future. We are currently not, and we wont be, so not much risk of later breakage.

We have a rule of thumb already: Acronyms up to 4 characters all uppercase, otherwise normal camelcase.

Now, it is often a matter of taste what is an acronym and what not and also not all acronyms have a clear casing, prime example mysql.

Being too nit-picky here is just a pain in the ass and we won't reach a common consensus anyway.

Comment by Roman S. Borschel [ 25/Jan/10 ]

Oh and we better dont start arguing about whats better to read because there is no chance of agreement. If you ask me I find SQLException better than SqlException. So here we go. Lets better not run down this path.

Comment by Glen Ainscow [ 25/Jan/10 ]

No need to get upset, I'm only trying to help.

I just thought that consistency is usually a good idea, either way.

I have to disagree in that an acronym is an acronym, it's not a matter of taste, the letters either stand for something, or they don't.

As for "mysql", only the SQL part is an acronym. So, MySQL or MySql.

Close if you disagree.

Comment by Roman S. Borschel [ 25/Jan/10 ]

I'm not upset. What made you think so? Maybe I should introduce a every now and then.

There's just no point in arguing about readability.

Like I said, we do have a naming standard even if its not adhered everywhere. The standard is also not something we've made up ourselves because we try to avoid that. When we introduced namespaces, we talked about adopting either the Java or .NET naming standards. We opted for the .NET standards. And there its recommended to write acronyms with up to 4 characters all uppercase.

I dont think there are too many violations of that in the code, probably Cli => CLI and some others but most of it looks ok to me.

UPDATE: Maybe we can gather a list here in this ticket of violations? Cli => CLI would be one. MySql => MySQL another. "Id" is rather an abbreviation of Identifier and not an acronym, to me at least.

Comment by Guilherme Blanco [ 25/Jan/10 ]

It's not a case of getting anyone upset...

But we should fix it and keep consistency of our codebase.

@romanb We should all talk and reach a common sense.
That's our last chance (last alpha) to get this consistency in.

Comment by Roman S. Borschel [ 25/Jan/10 ]

@Guilherme: We do have a naming standard since a year. You want to change the standard now?

Comment by Glen Ainscow [ 25/Jan/10 ]

@Roman,

Just a feeling I got.

This issue was more about consistency (indicated in the title) than moving to proper-case naming.

I think it might be up to 3 characters in .NET, HtmlElement, System.Linq, etc. Anyway, not important.

I agree that Id. is an abbreviation.

There are some more violations. If you decide you want to change them, let me know and I'll compile a list.

Comment by Roman S. Borschel [ 25/Jan/10 ]

@Glen: Yes, a list would be great. I find it hard to be 100% consistent sometimes though because my taste conflicts with the rule. For example, I would prefer "Id" over "ID", especially since it comes directly after ORM "Doctrine\ORM\ID\..." would be a bit too much. But I would not like "Orm" or "Dbal" either. But I think in most cases we can clearly fix the inconsistency. Like CLI or MySQL or MSSQL (or should it be MsSQL?).

Thanks for your help!

We should probably create updated coding standards for Doctrine 2 also, since they differ quite some from Doctrine 1 due to the introduction of namespaces and such. I'll create a ticket for that.

Comment by Glen Ainscow [ 25/Jan/10 ]

I'll do the list for you by tomorrow at the latest, just running out of time ATM.

Id is correct, as mentioned above, so that would be fine. MsSQL is correct (Ms = Microsoft = abbreviation).

Comment by Glen Ainscow [ 25/Jan/10 ]

Found some time ...

Doctrine\Common\Cache\ApcCache -> APCCache
Doctrine\Common\Cli -> CLI
Doctrine\Common\Cli\Printers\AnsiColorPrinter -> ANSIColorPrinter
Doctrine\Common\Cli\CliController -> CLIController
Doctrine\Common\Cli\CliException -> CLIException

Doctrine\DBAL\Driver\PDOMsSql -> PDOMsSQL
Doctrine\DBAL\Driver\PDOMySql -> PDOMySQL
Doctrine\DBAL\Driver\PDOPgSql -> PDOPgSQL
Doctrine\DBAL\Driver\PDOSqlite -> PDOSQLite
Doctrine\DBAL\Logging\EchoSqlLogger -> EchoSQLLogger
Doctrine\DBAL\Logging\SqlLogger -> SQLLogger
Doctrine\DBAL\Platforms\MsSqlPlatform -> MsSQLPlatform
Doctrine\DBAL\Platforms\MySqlPlatform -> MySQLPlatform
Doctrine\DBAL\Platforms\PostgreSqlPlatform -> PostgreSQLPlatform
Doctrine\DBAL\Platforms\SqlitePlatform -> SQLitePlatform
Doctrine\DBAL\Schema\Visitor\CreateSchemaSqlCollector -> CreateSchemaSQLCollector
Doctrine\DBAL\Schema\Visitor\DropSchemaSqlCollector -> DropSchemaSQLCollector
Doctrine\DBAL\Schema\MsSqlSchemaManager -> MsSQLSchemaManager
Doctrine\DBAL\Schema\MySqlSchemaManager -> MySQLSchemaManager
Doctrine\DBAL\Schema\PostgreSqlSchemaManager -> PostgreSQLSchemaManager
Doctrine\DBAL\Schema\SqliteSchemaManager -> SQLiteSchemaManager
Doctrine\DBAL\Tools\Cli -> CLI
Doctrine\DBAL\Tools\Cli\Tasks\RunSqlTask -> RunSQLTask

Doctrine\ORM\Mapping\Driver\PhpDriver -> PHPDriver
Doctrine\ORM\Mapping\Driver\XmlDriver -> XMLDriver
Doctrine\ORM\Mapping\Driver\YamlDriver -> YAMLDriver
Doctrine\ORM\Query\Exec\AbstractSqlExecutor -> AbstractSQLExecutor
(Should Doctrine\ORM\Query\Expr\Andx and Orx be AndX and OrX?)
Doctrine\ORM\Query\SqlWalker -> SQLWalker
Doctrine\ORM\Tools\Cli -> CLI
Doctrine\ORM\Tools\Cli\Tasks\RunDqlTask -> RunDQLTask
Doctrine\ORM\Tools\Export\Driver\PhpExporter -> PHPExporter
Doctrine\ORM\Tools\Export\Driver\XmlExporter -> XMLExporter
Doctrine\ORM\Tools\Export\Driver\YamlExporter -> YAMLExporter

... I think that's it. More than you expected? I did say: "There is more proper-casing than upper-casing."

Comment by Roman S. Borschel [ 25/Jan/10 ]

No, not more than I expected. It's mostly SQL and CLI basically. Thanks for the list!

We should try to go through that list before beta.

Comment by Roman S. Borschel [ 05/Mar/10 ]

Methods should be adjusted accordingly also (even though they're case insensitive). I already started with that. More to come.

Comment by Roman S. Borschel [ 28/Mar/10 ]

Most of the Cli => CLI changes seem to be complete now.

Comment by Roman S. Borschel [ 26/Apr/10 ]

Pushing the rest of the name changes towards beta2.

Comment by Roman S. Borschel [ 19/May/10 ]

More renamings have been done but still more to do. Pushing remaining work to beta3.

Comment by Roman S. Borschel [ 30/Aug/10 ]

Final name changes should be done prior to going into RC1.

Comment by Benjamin Eberlei [ 31/Oct/10 ]

there is much to be done yet, however most of it is also public API class names and might cause quite some BC breaks (i.e. DBAL Platforms and Schema Manager, Mapping Driver Names). I am not sure how to proceed here.

Comment by Roman S. Borschel [ 31/Oct/10 ]

Since PHP is mostly case-insensitive on class and method names it shouldn't be much of an issue.

Comment by Guilherme Blanco [ 16/Apr/14 ]

Scheduled to 3.0 as this may potentially create BC breaks





Generated at Thu Apr 24 04:35:56 UTC 2014 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.