[policy, no patch] Revised standards for class naming within namespaces

I quite like #4, it's a bit more greppable than aliasing, and looks like in some situations it could mean less use statements overall at the beginning of files.

1) Is a good solution, but it will lead to a non consistent codebase, and would imply we close this bug, so I will just ignore it.

2) I would not encourage class names to be always redundant with the namespace, but one word class names are often really bad.

3) Using too much aliasing would make the code harder to grep/refactor. Greping on Namespace\Classname would always gives us positive files matching the class, but wouldn't give the lines where it's being used, thus defying the purpose of grep itself.

4) Starting to use relative namespaces everywhere as a convention is a no go in my opinion. I'm quite sure it would be acceptable sometime, but only in rare cases (and I honnestly can't get my mind to find one example right one).

Aliasing is legal by the language, it's an helper to solve ambiguities, not something we must abuse of, in most cases it changes class names in the local context and make the code harder to read if used too much (once the class is named Database, another CacheDatabase, some people might even alias it as DatabaseCache) it makes "moving" class names and give some kind of random aspect to the code, which is the opposite of being explicit.

I'm not fond of repeating the full namespace into class names, but considering we did use Drupal\Core\Cache\*; upper in the code.

I would read this:

$joe = new Database();

"Joe is a new database", which for my mind would make think (even if I know it inst) that Joe is a new database connection. Even if I know by reading the use statement upper Joe is not a database, but at first sight, I don't know what Joe is and my mind would tell me instinctively the wrong thing.

Then if I read this:

$joe = new DatabaseDriver();

"Joe is a new database driver", ok, it's better, but once again, my mind is fooling me around, I would think that Joe is a database driver (is it MySQL, PostgreSQL, any other?), but once again, my mind will instictively read this wrong.

Now, if I name my class with some more redundancy, I get this:

$joe = new DatabaseCacheDriver();

"Joe is a new database cache driver", this sounds a lot more explicit, and my mind did not instictively fool me! I know it's a long class name, but it is an explicit class name, and 3 words is not too much to write, especially when you have a good IDE (and it doesn't lead to extremely long code lines either).

What I mean by that is that somehow namespaces are primarily a medium to create encapsuled packages and avoid conflicts between frameworks themselves. It's not just about making class names smaller. Anyway, we are supposed to put interfaces anyway, and interfaces names will be smaller in most cases.

We should repeat words and allow redundancy in favor of readability. It's common practice so I'm not sure there is a lot to debate here.

See also #1323124: Convert file transfer system to PSR-0 and follow-up comments. As effulgentsia pointed out in that issue, it is consistent with Symfony. For example, Symfony\Component\HttpFoundation\SessionStorage\ArraySessionStorage. It would be confusing to use Array instead of ArraySessionStorage.

I am a fan of #2 (less ambigous, more stand-alone class names), mostly from the point of view of searching/browsing on api.drupal.org. As Dries notes in #4, Symfony does this already...

I don't think we want a bunch of too-short-to-understand class names sitting there if we go to a page like
http://api.drupal.org/api/drupal/classes/8
or if you go over to the search box and start typing in something like Array, and there are 10 classes called just Array, it would be hard at first glance to figure out which one was wanted.

Definite hyperbole. :) Let's see if we can make something sensible... How about this for a standard:
---
Classes should have names that stand alone to tell what they do (without having to refer to the namespace), and are as short as possible without losing information about what they do or leading to misunderstandings.

Examples:

---

#7 looks great.

RE #9 - I am not sure they all need to be uniformly prefixes/suffixes. I took those from actual examples in Core and tried to make what I thought were sensible class names in all cases, that would be what you would call them in plain English:
- QueryCondition: "This class represents a query condition"
- LocalFileTransfer: "This class takes care of local file transfers"
- DatabaseCacheDriver: "This class is a database cache driver" (at least that was my guess as to what it does from the comments above).

I don't think we need any rules on how to make up class names... "use common sense" will probably be fine. I think? But we can adopt standards for prefix vs. suffix if that makes things more uniform, and if that would be better than "use your own judgement".

#7 seems real good to me.

@#6

Drupal\Core\Database\Query\Condition. "Condition" means little out of context, but with the full name it's completely obvious.

The namespace is only a container for organizing your code, you don't use the namespace, you use the class name when you code.

Remember that the full string is the class name as far as any PHP internal code knows. The rest is just syntactic sugar.

PHP use an internal class name with the full namespace, we don't. Every other high level OOP language that runs on a VM does the same.

@#9

Well, that leaves it unclear how to form that name.

Thanks to namespace context encapsulation that provides for us a clean code organisation (as if it where folders), we can now afford to use native english language to form class names in order to make the names both explicit and naturally readable, that's how we can now form the names (that we couldn't do when we had no namespaces). We can basically read the class names as incomplete sentences and naturally understand their meaning even without knowing the whole design (except in some complex APIs). Of course we would never call a class "TheClassThatDoesTransferFileLocally" but "LocalFileTransfer" instead, but that's just having a bit of sense :)

I'm OK with making a standard for how to name the classes, and I agree that having a clear standard definitely shuts down bikeshedding quickly. ... Maybe a clear *guideline* with some flexibility would be best, though, in case there are exceptions that would make sense. So how about this as an alternative to the proposal I put in comment #7:

---
Classes should have names that stand alone to tell what they do (without having to refer to the namespace), and are as short as possible without losing information about what they do or leading to misunderstandings. Normally, they should be formed from the last component of the namespace, followed by a word or words that gives the specific function of the class.

Examples:

---

How's that?

OK:
---
Classes should have names that stand alone to tell what they do (without having to refer to the namespace), and are as short as possible without losing information about what they do or leading to misunderstandings. Normally, they should be formed from the last component of the namespace (if needed to disambiguate), followed by a word or words that gives the specific function of the class.
---
Did I spell "disambiguate" there right? :)

And are there any examples of core where we wouldn't need to add the namespace to add to the examples table?

Hm. So there are several classes called just "Select"? Uck. That violates the "unambigous" idea pretty clearly. I would think MySqlSelectQuery would be a less ambigous name... but if that wouldn't work, it wouldn't work.

Agreed on "User", "Node", etc. as good examples where the namespace isn't needed.

I'm glad that everyone seems to lean toward the same opinion here. I agree that for the database driver it might cause technical problems due to the current architecture, it's an edge case, IMO it seems that the database driver has a really weird API, we could have an interface for each driver that would implement methods such as getSelectQuery() and the naming problem would be solved.

Yes agree, anyway the database system is an edge case and I'm not sure we should use its driver system as example for naming conventions.

Agree with that 100%.

OK, one more try at a standard:

---
Classes should have names that stand alone to tell what they do (without having to refer to the namespace), and are as short as possible without losing information about what they do or leading to misunderstandings. Normally, they should be formed from the last component of the namespace (if needed to disambiguate), followed by a word or words that gives the specific function of the class, and they should not include "Drupal" or "Class" in the name (although interfaces can include "Interface" in the name).

Examples:

---

Note: I didn't find a class named "Node" in D8, and it doesn't look like the Entity class currently has a namespace (???).

I would go for less strict, maybe: Class name must be short, but obvious about what it does or represent. It may contain the namespace name in order to prevent any confusion when being used outside of its own namespace. If possible, make them as near as possible of spoken english. Code using them must be explicit and naturally readable if possible. along with a few but self-explenatory examples.

The ideas in #25 vs. #26 - read the comments above. #25 has adapted to many comments that disagree with #26.

crell: You were the one who brought up many of the problems (inconsistency, etc.) with the wording I had proposed originally (see #8), and examples that didn't have the right order (see #9), right? I personally really don't care strongly, but I just want to make sure that all the good ideas other people had are represented in the standards.

@#27 I volontarily make my text more evasive, because the more strict are the standards, the more edge cases becomes incompatible with it and become themselves new troll about standards; The more we have standards to follow the harder it becomes to simply follow them when we code or review. There is one thing that I hate in core issue queues, is that in most threads, more posts actually speaks of standards violations using Dreditor review instead of speaking of the real code and design. We should easier everyone's life with standards, we cannot anticipate all use cases in advance.

Right, I understand that pounard, but I would just point out that several people objected to my similarly vague first proposal in #7, which is how (after several iterations) we arrived that the more-specific proposal in #24.

Really, I don't care which way we go, but after having a lot of discussions and making it more specific in response to those discussions, it seems odd to suddenly say "Oh, let's go back to being vague" again. Really, the proposal in #25 has pretty much the same information as the one I suggested in #7... Read through the responses to understand why we arrived at #24...

The problem is that stuff like Normally, they should be formed from the last component of the namespace [...] followed by a word or words are not always true, the repeated namespace piece would be the second or the third word, in some case. I just don't want this kind of things to happen once this as been written in the standards, because if it does, it will re-open the troll once again, and ends up in a gigantic waste of time, again.

EDIT: Plus, I'm quite sure that for any core code review they will be people skilled with namespaces that will expose their thoughts and reveal the naming nonsense, it already happened more than once, and it almost always end up by good names.

OK, let's recap, because I'm confused about who likes what version of the proposed specs....

1. I think everyone agrees on the concept that the class names should be sufficient to stand alone and describe concisely what the class does (and that some of the existing/proposed class names will therefore need to be revised).

2. I proposed a simple "use common sense" type of statement of this, with three examples that basically followed the "how would you say it" order, in comment #7. Several people said they liked it, but Crell objected in #9 that this proposal gave no guidance on how to make the names (what order), and that the three examples used three different ways of combining the namespace with the rest of the name. And added in #12 that it would be better to give some guidance in the standard to avoid bikeshedding on class name choices.

3. I proposed a new standard in #13 that gave guidance (namespace name followed by specifics), with revised examples. Crell proposed in #14 amending to say only to use the namespace name if necessary to make it unambiguous, and in #22 to say not to use "Drupal" in the name (and there was some more concurring discussion on these two points).

4. I proposed a new standard in #24 that included these ideas, and added an example showing not to use Drupal in the class name.

5. In #26, pounard proposed a "less strict" standard (basically going back to the "common sense" approach from #2, with slightly revised wording), discarding the arguments from #9/12 about the wisdom of having guidance on how to revise the name. Crell stated in #28 that he prefers #26 to #24 (with an addition to say not to use Drupal in the name), and then clarified in #33 that what he preferred was the idea that adding the namespace to the name should not be percieved as the default.

So....

Given all of that, I hereby make one more proposal, which keeps the idea of giving specific guidance, but does not imply that you should include the namespace name in most cases. How's this?

------
Classes should have names that stand alone to tell what they do without having to refer to the namespace (if it is necessary for clarity or to prevent ambiguity, prefix the class name with the last component of the namespace). Class names should be as short as possible without losing information about what they do or leading to misunderstandings, and should not include "Drupal" or "Class" in the name (although interfaces can include "Interface" in the name). Also note that due to the way database classes are loaded, even though it would prevent ambiguity, do not attempt to include the database engine name (MySQL, etc.) in engine-specific database class names.

Examples:

---

I think mentioning the database stuff is necessary, because as I see it, it's a violation of the "unambiguous" part of the standard. How's this for the intro:

---
Classes and interfaces should have names that stand alone to tell what they do without having to refer to the namespace, and are as short as possible without losing functionality information or leading to ambiguity. Notes:

• If it is necessary for clarity or to prevent ambiguity, prefix the name with the last component of the namespace.
• Names should not include "Drupal".
• Class names should not have "Class" in the name.
• Interface names should end with "Interface".
• Due to the way database classes are loaded, even though it would prevent ambiguity, do not include the database engine name (MySQL, etc.) in engine-specific database class names.

---

I like #36, but I don't like If it is necessary for clarity or to prevent ambiguity, prefix the name with the last component of the namespace.: LocalFileTransfer actually use the namespace name as suffix, not prefix. It should probably be If it is necessary for clarity or to prevent ambiguity, you can repeat the last component of the namespace in the class name.

Aside, the DB parenthesis may be refered as a technical exception, not as a rule, such as: Database driver classes violate this standard due to technical reasons and follow their own standard, refer to the documentation for details.

Otherwise, it seems good to me.

RE #38 - we don't want to use LocalFileTransfer. If you look at the examples table (in #34), we decided (see recap in #36) that we wanted to have consistency and use a prefix, so it's supposed to be FileTransferLocal.

FileTransferLocal sounds less english and less naturally readable than LocalFileTransfer, I don't understand why this has to be the prefix? I really don't agree with it, it will imply poor names in a lot cases while the whole goal is to make clear and nice class names (yes, I said nice, because it's also nicer to read and write, sounds like a real sentence):

// My variable is a new local file tranfert.
$my_variable = new LocalFileTransfer();
// Vs.
// My Variable is a new file transfer local. <= poor english
$y_variable = new FileTransferLocal();

RE #40 - conflicts with Crell's comments in #9/#12 about having something definite to reduce bikeshedding.

My feeling is that if the file transfer classes are called FileTransferLocal, FileTransferRemote, etc., that is just as good as LocalFileTransfer, RemoteFileTransfer, etc. [I made up the "remote" one]. Also, that would have the added benefit that on api.drupal.org, they would all be near each other in the alphabetical list of classes. And I think in most cases, the "prefix with the namespace" rule makes for good reading order anyway... having a standard, and grouping classes together in alphabetical order, makes up for that small readability hit.

So we have a difference of opinion here. Any other votes/ideas?

RE #41 - Symfony appears to be all over the map as far as class names, and not to prefer a prefix. Maybe we shouldn't either.

I personally don't feel *too* strongly about it, but I think (as stated in my previous comment) that prefixes do carry some advantages (reduce bikeshedding, alphabetical grouping) at the (maybe) cost of a small amount of readability (and even that is debatable).

The natural english feels right in the context where classes that belong together are in the same namespace, so reducing sort problems to small encapsulated namespaces, I am sure that in that context, making classes closer to english seems a good tradeoff.

@#43 About naming classes so they are displayed altogether seems natural when you don't have a namespace, but here we have, and I hope that api.drupal.org listings will list fully qualified names, else it will cause other problems than just sorting, potential name conflicts (even if we do this naming convention in order to avoid it, it's still possible we may end up with some, it doesn't feel always wrong).

I think that going with natural english will solve many of the potential bikesheds the we leave open by not forcing the namespace segment position, except if we leave the decisions to people that don't speak english at all :)

OK. Hopefully final proposal (and I'd like to votes from others besides Crell and poundard on this if possible)... I added "read well" to the main sentence, and took out the "prefix" namespace idea, and slightly reworded the database classes exception:
---
Classes and interfaces should have names that stand alone to tell what they do without having to refer to the namespace, read well, and are as short as possible without losing functionality information or leading to ambiguity. Notes:

• If it is necessary for clarity or to prevent ambiguity, include the last component of the namespace in the name.
• Names should not include "Drupal".
• Class names should not have "Class" in the name.
• Interface names should end with "Interface".
• Exception for Drupal 8.x: due to the way database classes are loaded, do not include the database engine name (MySQL, etc.) in engine-specific database class names.

Examples:

---

Note: I was going to add an example for core/lib/Drupal/Core/Database/Driver/mysql/Select, but I really think that class should be called SelectQuery (since "select" is also a type of HTML element), so I left that out rather than starting a bikeshed discussion here.

And if you want to have influence on how api.drupal.org handles namespace, discuss on this other issue in the API project:
#1507476: Support namespaces in API module

Thanks for the typo in my nickname :) Else it sounds better, it contains everything we need, I think. I'm not sure that without having to refer to the namespace is necessary because the first bullet speak for itself. Aside of that, I'm OK.

sorry for typo, can't type today...

Hehe don't worry, as I said, I could live with the actual text.

I like #48.

To add to #44, while those examples all show subnamespace as postfix, an example of prefix is Symfony\Component\HttpKernel\Controller\ControllerResolver, and an example of infix is Symfony\Component\HttpFoundation\File\MimeType\FileinfoMimeTypeGuesser. So +1 to disambiguating with as natural a name as possible, rather than being too rigid with where to incorporate the subnamespace name. If it looks like in future issues, people bikeshed over whether a prefix, infix, or suffix is more natural, we can add some additional rules (like incorporating distinctions between adjectives and nouns).

I really think that class should be called SelectQuery (since "select" is also a type of HTML element), so I left that out rather than starting a bikeshed discussion here.

I agree, but am happy to leave that discussion to a followup if it's controversial.

Rereading the comments here, seems like everyone's in agreement, including Dries and webchick, so RTBC. No word from catch since #2, but maybe having this in the RTBC queue will change that.

OK, let's leave this at RTBC for 3 days or so, and then edit coding standards to add #48 to it, if no one objects.

Updating title, and right after saving this comment I will update the issue summary.

If this goes in can we reverse that minor release version raise we did the other day? Please.

I've been following this and the plan seems fine to me fwiw.

I have added this information to the Naming section of http://drupal.org/node/608152

So... We now have the policy in place. Do we now need a patch, or should we mark this issue fixed and start another one for the patch (if we even need one)?

OK then, tentatively marking this fixed, pending anyone taking issue with how I added the info to http://drupal.org/node/608152 [not **exactly** as above, as I incorporated it into existing naming section]

Please see #1477218-34: Convert Tracker tests to PSR-0 / PSR-0 test class discovery and below for questions on how to apply this new standard to test case classes.

For anyone still following this issue...
#1809930: [meta] Many core class names violate naming standards
Quite a few of our classes are currently violating these standards. Sigh. Some were even changed recently apparently... discuss on that issue please, this one is closed.

Since the time when this standard was officially adopted, what has actually happened is that most of D8 core has not been following it. So, a proposal is underway to revise the standard to be more in line with what we're actually doing, on:
#2027221: [policy] Revisit class naming standards

add summary