[DDC-274] Class and namespace naming inconsistency Created: 24/Jan/10  Updated: 26/Jun/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: Major
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 Oct 23 01:37:38 UTC 2014 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.