Problem/Motivation

We need to decide on whether to use CamelCase or ALLUPPER for acronyms in Drupal Core class, interface, and namespace names.

The standard adopted on this issue earlier (around comment #33) was that they should be ALLUPPER. Various people prefer one alternative or the other (just based on personal preference), and other PHP projects are all over the map on which standard they prefer.

So, our options are:

a) Keep the standard currently in our naming standards documentation, which is "Acronyms must be all caps". [This causes various problems (see comment #65) and leads to readability issues (see #47).]

b) Revise it slightly, such as "Acronyms must be all caps. Don't put two acronyms in a row." or some other concrete suggestion(s) that would increase readability for specific circumstances. [Note that we don't have a concrete proposal for what this would look like, and it's hard to imagine one that would be easy to enforce and understand.]

c) Reverse the current standard, and say "Acronyms should also use CamelCase." [The only problem here is that some people don't like it.]

d) Not have a standard at all. [Leads to bikeshedding and code non-uniformity.]

Proposed resolution

Use CamelCase for all acronyms in class, interface, and namespace names. [option (c) above].

Examples:

Drupal\Component\Uuid\Php
Drupal\field\Tests\FieldInstanceCrudTest

Remaining tasks

  1. Agree upon a standard.
  2. Document it.
  3. Fix the classes that don't match. (Either a handful, or a lot.)

API changes

Whichever classes don't match the agreed-upon standard get renamed.

Edit: 2019-10-23

The consensus seem to be that we are moving (have moved already) to camelCase for acronyms in names. See comments #71 and #86

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

aspilicious’s picture

.

merlinofchaos’s picture

Just my personal opinion:

While I understand the arguments for re-casing acronyms, I find it annoying. The two primary arguments are:

When lowercasing the beginning of something, such as PHPCamelCasedThing, you end up with pHPCamelCasedThing which is ugly (though in reality you end up with phpCamelCasedThing which is fine) and that you camel case oddly if you have something like PHPYAMLCamelCasedThing but those things are rare in our code base and unlikely.

In short: I think Php just looks wrong over PHP and the standard there is controversial to begin with.

Tor Arne Thune’s picture

In my opinion, the ultimate goal is to make the code more readable. We are used to acronyms like PHP and ID, which make it strange to see these camelcased. On the other hand camelcase is also used to make it easier to see where one word starts and another begins, without having to use another character as delimiter (usually underscore). NodeRSSContentTest is a little bit harder to scan because of this, but keeps the acronym we all know and love.

So there are benefits and disadvantages to readability.

Tor Arne Thune’s picture

Also, as xjm mentioned, when the Camelcased acronym accidentally makes a word when not capitalized fully it can make someone think it means something else. As mentioned in the issue summary: FilterCrudTest (a test that checks for crud in the filter module anyone? ;)).

RobLoach’s picture

Acronyms are camel-cased. Some examples in Symfony:

To understand the reasoning behind this, we can refer to CamelCase in Programming/Coding. In it, you see:

Programming identifiers often need to contain acronyms and initialisms which are already in upper case, such as "old HTML file". By analogy with the title case rules, the natural camel case rendering would have the abbreviation all in upper case, namely "oldHTMLFile". However, this approach is problematic when two acronyms occur together (e.g., "parse DBM XML" would become "parseDBMXML") or when the standard mandates lower camel case but the name begins with an abbreviation (e.g. "SQL server" would become "sQLServer"). For this reason, some programmers prefer to treat abbreviations as if they were lower case words and write "oldHtmlFile", "parseDbmXml" or "sqlServer".

For this reason, you understand why acronyms become camel-cased. The difference between what Wikipedia has and our coding stanard though, is that Properties get lower-case camel cased, and classes/namespaces get the upper-case camel-case. We have been following this in Drupal 8 core pretty well so far (new Uuid(), not new UUID()).

There are a few tests that we missed the camel-casing of acronyms for during the PSR-0 transition. We could probably clean those up in this issue.

Tor Arne Thune’s picture

Just to be clear (as I've heard some misunderstandings around this): PSR-0 says nothing about camel-casing. PSR-1 does though, but we haven't decided to adopt that.

merlinofchaos’s picture

I think it unlikely we will adopt PSR-1.

I already attempted to refute most of #5 in #2. I find those arguments uncompelling.

The disadvantages and advantages mentioned in #3 are the ones I see.

I grew up in CamelCase using C++ and all the projects I ever worked on did not lowercase acronyms. I've seen many, many subtle variations on camel case. There are differing standards depending upon the project and who you speak to.

There is an argument to be made that we should adopt the standard Symfony uses because we're using Symfony.

There is an argument to be made that the most important thing is readability.

jhodgdon’s picture

+1 for choosing and adhering to a standard.
+1 for not arguing about it much. There are good reasons for both possibilities, as seen above. Let's just choose one or the other and call it done.

My vote would be to choose the one that the majority of our existing Drupal classes use, to minimize the patch size. I am not going to weigh in on "I like this one better" (even though I do actually have an opinion), since I agree that there are aesthetic arguments on both sides and I don't want to keep the debate open or have endless bikeshedding.

aspilicious’s picture

Status: Active » Needs review
FileSize
8.48 KB

Patch size isn't an isse IMO. I illustarte this with a patch that converts everything to the camelcase standard. The other way around will produce a bigger patch but not that much bigger.

I can make that one too if needed. This is just to show patch size isn't an issue...

aspilicious’s picture

Is that correct? I have the feeling my file renames aren't processed...

EDIT: yeah file renames are missing

RobLoach’s picture

FileSize
9.18 KB
aspilicious’s picture

We probably also need a patch with the opposite approach. To compare those...

tim.plunkett’s picture

Status: Needs review » Needs work

I'm conflicted in reviewing this, because I think it's a terrible idea, and there is nothing uglier than seeing "Ui" in class names. That said...

+++ b/core/lib/Drupal/Core/FileTransfer/FtpExtension.phpundefined
@@ -10,7 +10,7 @@ namespace Drupal\Core\FileTransfer;
+class FtpExtension extends FTP implements ChmodInterface {

`extends Ftp` then, I guess

+++ b/core/modules/openid/lib/Drupal/openid/Tests/OpenIdFunctionalTest.phpundefined
@@ -12,7 +12,7 @@ use stdClass;
+class OpenIdFunctionalTest extends OpenIDTestBase {

Wouldn't this be OpenIdTestBase, here and elsewhere throughout

sun’s picture

I totally agree with @merlinofchaos, @xjm, and @tim.plunkett.

Acronyms should not use camel-casing in class names and namespaces. That's a completely unnatural way of writing. The reasoning provided in the OP actually is the most compelling one and just simply makes sense:
This is logical since each letter of the acronym is the beginning of a word.

Writing code that uses camel-casing for acronyms requires additional braincells for just writing class names. I just experienced that to a great extent in #1637370: Add UUID support to core entity types — writing $entity->uuid = new Uuid(); is crazy. Try it yourself. Even after writing it a couple of times for that patch already, it just again took me 4 times to get that Uuid right. What I continue to end up with is $entity->uuid = new Uuuid();. Contrary to that, writing $entity->uuid = new UUID(); is just natural.

Looking at Symfony is no argument for me either, as their coding standards are different to ours in many other ways. In general, there is no universal standard for this detail. Even though the issue exists not only in PHP, but also in other object-oriented languages, you will have a hard time to find any majority for either style.

PHP itself is not very consistent in itself, but at least most of the object-oriented libraries are retaining acronyms in uppercase in class names and also method names:
http://php.net/manual/en/book.dom.php
http://php.net/manual/en/class.pdoexception.php
http://php.net/manual/en/class.xmlreader.php
http://php.net/manual/en/class.simplexmlelement.php
(there are many more examples in PHP, and I thought it would be easy to find counter-examples in PHP, but apparently I didn't find any in my quick search)

So if "do what others do" is an argument at all, then we should follow PHP's style.

jhodgdon’s picture

#14 ==> +1 (for once I agree completely with sun.) :)

xjm’s picture

I find #14 compelling as well.

tim.plunkett’s picture

I was going to get to work on this, but I won't have time right now.

So instead I've gone through and come up with a list of incorrectly capitalized acronyms in classes/interfaces:

  • Api
  • Crud
  • Efq
  • Ftp
  • Gd
  • Html
  • Http
  • Id
  • Ip
  • Js
  • Json
  • Lob
  • Mime
  • Opml
  • Pecl
  • Php
  • Rdf
  • Rpc
  • Rss
  • Sql
  • Ssh
  • Ui
  • Url
  • Xml
  • Xss

Also, shouldn't this be extended to methods as well?

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
57.69 KB

Okay, got part way done.

  • Api
  • Crud
  • Efq
  • Ftp
  • Gd
  • Html
  • Http
  • Ip
  • Js
  • Json
  • Lob
  • Mime
  • Opml
  • Pecl
  • Php
  • Rdf
  • Rpc
  • Rss
  • Sql
  • Ui
  • Url
  • Xml
  • Xss
tim.plunkett’s picture

FileSize
92.45 KB

Because of all of the instances of HttpKernel and UrlMatcher in Symfony, I've left all occurrences of those mis-capitalizations in place.

The rest should be taken care of.

aspilicious’s picture

Still not fan of "XMLRPCBasicTest" for example but I see there are lots of people into this idea so I'm not going to be the bad guy. Btw you also should convert Uuid => UUID

xjm’s picture

Status: Needs review » Needs work

So I was going to add in the UUID changes, but: http://blog.bauffman.be/2011/09/07/changing-filename-case-in-git/

Apparently the solution is to either 1. Apply the patch on Linux or 2. Create two patches, one that renames everything to FooABCWhatever.phptmp and then one that strips off the tmps.

Trying on a Linux box now since I think @jhodgdon uses Linux anyway.

tim.plunkett’s picture

Duh I forgot uuid!

And yes, I had to do the two step process.

Tor Arne Thune’s picture

Status: Needs review » Needs work

Also, we have Tpl in some places that refer to template files. Do we want to change those too? It's not an acronym, but neither is PHP (well, it's a recursive acronym), XML and XSS, so...

tim.plunkett’s picture

Status: Needs review » Needs work

I saw that and decided abbreviation != acronym and left it

xjm’s picture

Status: Needs work » Needs review
FileSize
8.07 KB
98.2 KB

Edit: Not sure why it's not detecting the rename for PECL.php; guess it's too short of a file so the similarity isn't high enough.

Tor Arne Thune’s picture

Status: Needs work » Needs review

@#24: I agree, let's not go down that road.

xjm’s picture

Status: Needs work » Needs review

Yeah, they are Templates, not TemPLates or TastilyPreparedLattes, so that one should stay as-is.

sun’s picture

+++ b/core/lib/Drupal/Component/UUID/Com.php
@@ -2,17 +2,17 @@
  * UUID implementation using the Windows internal GUID extension.
  *
  * @see http://php.net/com_create_guid
...
+class Com implements UUIDInterface {

COM is an acronym as well; stands for the Component Object Model on Windows.

However, I also think we can handle missing/remaining names in follow-ups. This patch doesn't look particularly easy to roll and re-roll...

tim.plunkett’s picture

FileSize
98.2 KB

Rerolled for Com/COM.

Reminder, this patch cannot be applied on OS X!

jhodgdon’s picture

Two questions:
1. Do we need to update the coding standards somewhere about this, and if so, can we say this has been decided and update the standards?
2. Regarding "Tpl" -- don't we have a standard somewhere that says not to abbreviate in function/variable/class names at all -- in other words, shouldn't this be written out as "Template" (probably a separate issue though)? (Hm. I thought we did, but I can't find it...)

xjm’s picture

Regarding #1, I am +1 for calling this decided and updating http://drupal.org/node/608152.

Regarding #2, http://drupal.org/coding-standards#naming doesn't specifically say words need to be written out, although it's sort of implied. tpl for template is already in use in our filenames, though, so that one I'm not sure about. I agree that's a separate issue.

jhodgdon’s picture

Title: Case of acronyms in class names (SomethingXSSClassName versus SomethingXssClassName) » [policy and patch] Case of acronyms in class names (SomethingXSSClassName versus SomethingXssClassName)

Regarding this standard being agreed to... looking at the comments above, it looks like we have:

Leaning in favor of all upper case for acronyms: (FooHTMLClass): [see argument in #14]
xjm, jhodgdon, merlinofchaos, Tor Arne Thune, tim.plunkett, sun

Leaning in favor of camel case for acronyms: (FooHtmlClass): [see argument in #5]
Rob Loach

I'm not sure which way leaning:
aspilicious

So I'd say the majority of people who have chimed in here are in favor. Any other opinions? Did I misrepresent anyone's position?

aspilicious’s picture

I *think* people like Crell are also in favor of camel case for acronyms. We should give him the oppurunity to respond before making the call. :)

jhodgdon’s picture

Title: [policy and patch] Case of acronyms in class names (SomethingXSSClassName versus SomethingXssClassName) » [policy and then patch] Case of acronyms in class names (SomethingXSSClassName versus SomethingXssClassName)
Status: Needs review » Reviewed & tested by the community

RE #33 - yes, indeed -- that is why I posted the list of people who had responded rather than editing the standards page. Actually, the surest way to get people to respond with opinions on coding standards patches is to mark them RTBC and wait for a couple of days to see what happens. :)

NOTE: THIS IS RTBC FOR THE STANDARD ONLY (I have not reviewed the patch).

Crell’s picture

Except I don't routinely troll the RTBC queue. Very few people do. :-) I only just discovered this from another issue it was linked from.

What's the actual proposal now? Can someone update the summary with whatever it is that is supposedly RTBC?

tim.plunkett’s picture

@Crell, I updated the "Proposed resolution" in the summary.

sun’s picture

Perhaps I'm misguided, but I think the proposal in the summary states the opposite of what the code example changes right within it are showing ;) (The code examples are right, but the proposed solution statement is not.)

For reasoning, see #14

Also worth to mention: #24, #27, and following: abbreviation (Tpl) != acronym (HTML), and the abbreviation shouldn't exist in the first place (Template).

sun’s picture

Issue summary: View changes

Clarify the proposed resolution

tim.plunkett’s picture

Duh, I wrote exactly the wrong thing. Fixed.

Crell’s picture

Hm. "Do as others do" is a generally compelling argument for me in cases where we don't yet have a standard.

PSR-1 doesn't actually address this issue. (It says classes in UpperCamel, but is mute on the question of acronyms therein.)

It looks like PHP itself does FOOBar, while Symfony does FooBar. So either way we're inconsistent with somebody.

That said... I expect most devs will be seeing Symfony classes a lot more than PHP native classes. Drupal has a tendency to build rather thick layers, and even though we're trying to reduce that it is still there. So I would actually lean toward FooBar, not FOOBar. It's also one less barrier for comingling Drupal and Symfony developers, which I think is far more likely than comingling Drupal and PHP core developers.

merlinofchaos’s picture

That said... I expect most devs will be seeing Symfony classes a lot more than PHP native classes.

I just don't think I agree with this. Especially since PHP native class names are why we're stuck with camelcase to begin with. Plus, we're trying to USE Symfony, not BE Symfony. The argument that we should do as Symfony does is simply because Symfony does it is kind of scary to me.

jhodgdon’s picture

Let's step back a moment:
- Some projects use camel case and some use upper case for acronyms (and some are even inconsistent within the project), so we can't really decide on that basis.
- Some people prefer to see things like EatsHTMLForBreakfastViaFTP and some prefer to see things like EatsHtmlForBreakfastViaFtp. Some people's sensibilities are offended by each of those examples.
- As stated nicely by Tor Arne Thune in comment #3: "We are used to acronyms like PHP and ID, which make it strange to see these camelcased. On the other hand camelcase is also used to make it easier to see where one word starts and another begins, without having to use another character as delimiter (usually underscore). NodeRSSContentTest is a little bit harder to scan because of this, but keeps the acronym we all know and love." and in #4: "Also, as xjm mentioned, when the Camelcased acronym accidentally makes a word when not capitalized fully it can make someone think it means something else."
- Rob Loach points out in #5 that upper case presents problems when two acronyms are back-to-back, and for lower-camel-case scenarios.

So... There are arguments on both sides... We really just have to pick one as our standard, and live with it. Some people are going to be unhappy no matter which one we choose, and each one has pluses and minuses.

In the comments above, I see:

ALL CAPS: merlinofchaos, tim.plunkett, jhodgdon, sun, xjm

CamelCase: Crell, Rob Loach

It looks like the people interested in this issue are favoring ALL CAPS by a slight margin... can Crell and Rob Loach agree to set aside their disagreement and adopt that as the standard?

catch’s picture

Title: [policy and then patch] Case of acronyms in class names (SomethingXSSClassName versus SomethingXssClassName) » Case of acronyms in class names (SomethingXSSClassName versus SomethingXssClassName)
Status: Reviewed & tested by the community » Needs work

I massively prefer ALL CAPS since the falsely camel cased acronyms just look completely wrong, so I'm glad this issue went that way.

Since it's not in PSR-1, if we really want to be consistent with Symfony, perhaps someone can file a pr to see if they'll consider changing theirs to match both Drupal and (most of) PHP itself?

Moving to CNW since this needs a patch now I think.

jhodgdon’s picture

I added a note to the standards page about this new standard:
http://drupal.org/node/608152
Do we do change notices for standards changes? If so we should do one.

sun’s picture

PR against PSR about this: https://github.com/php-fig/fig-standards/pull/48

(And, FWIW, another PR against PSR on constants: https://github.com/php-fig/fig-standards/pull/47)

neclimdul’s picture

I actually don't see what the PSR's have to do with this. My understanding of StudlyCaps as used by PSR-1 just means "do something CamelCase'ish" so unless my understanding is wrong, either choice works. That means we just differ from Symfony and that's fine, we're different projects.

Crell’s picture

Just an additional data point here, it looks like FIG is close to releasing its first code-providing (well, interface providing) PSR, and will likely be using Fig or Psr in the namespace, not FIG or PSR.

Anonymous’s picture

When we start having acronyms with other acronyms, readability just starts getting horrible. For example, JSONLDEntityNormalizer vs JsonLdEntityNormalizer. Also, JSON-LD libraries written by the JSON-LD CG use JsonLd.

When I first researched this, I found a standard (I believe Microsoft internal) that any 2 letter acronym (or maybe it was <= 3) was uppercase, and longer acronyms are camel case. I think we should consider a sensible character limit for this rule.

jhodgdon’s picture

Another possibility for acronyms with acronyms is to alternate case, such as JSONldEntityNormalizer.

Crell’s picture

jhodgdon: That would be even more confusing and inconsistent with the entire rest of the PHP universe, as well as itself.

quicksketch’s picture

It looks like the people interested in this issue are favoring ALL CAPS by a slight margin... can Crell and Rob Loach agree to set aside their disagreement and adopt that as the standard?

I'd just like to add my support to the ALL CAPS team. Coming from a strong JS background, that language specifically uses camelCase with uppercase acronyms for their methods and variables.

i.e. document.getCSSCanvasContext or element.innerHTML

Interestingly, "Id" is not all uppercase, I'm guessing because it's really an abbreviation rather than an acronym. i.e. document.getElementById().

quicksketch’s picture

Issue summary: View changes

Brain fart.

Crell’s picture

PHP and JS coding standards are already quite different in a lot of ways. I still believe that consistency with most of the rest of the PHP world, including PHP-FIG, is more important than consistency with Javascript, at least when talking about PHP code.

quicksketch’s picture

In any case increasing consistency between JS and PHP code is a bonus in my mind; less modification of IDE and text editor settings is required when switching between the two. Developers of other PHP platforms are likely to also be developing JS code to facilitate their front-end, so even if they're not interested in being consistent with other PHP projects, they may lean towards the standards set by JS because they are more consistent. So even if "be like other PHP projects" is our priority, other projects may already be influenced by standards set by JavaScript, either directly or indirectly.

Crell’s picture

quicksketch: Have any evidence of that? The PHP-FIG group has recently standardized on SomethingXssClassName for all of its code.

quicksketch’s picture

After Googling around, I can't find any explicit evidence that PHP developers have been directly influenced by the precedents set by JS. Personally, I'm influenced it, so that's why I gave my +1 in #50.

Wim Leers’s picture

ALL CAPS: +1.

Though linclark makes an excellent point in #47. Maybe the rule should be: "ALL CAPS as a general rule, unless legibility is impeded" or "ALL CAPS except when there is a dash in the acronym".

Crell’s picture

I believe the only non-bikeshed/personal opinion data points that have been brought up are:

- Javascript uses SomethingXSSClassName.
- PHP internals uses SomethingXSSClassName
- Symfony uses SomethingXssClassName
- PHP-FIG uses SomethingXssClassName

Are there any other objective data points to bring up (meaning not "I like X better", for any given definition of X)?

jhodgdon’s picture

Title: Case of acronyms in class names (SomethingXSSClassName versus SomethingXssClassName) » [policy, then patch] Case of acronyms in class names (SomethingXSSClassName versus SomethingXssClassName)
Priority: Normal » Critical

We desperately need to resolve this issue NOW, so that we can proceed with cleanup, such as
#1809930: [META] Many core class names violate naming standards

pounard’s picture

I think it doesn't really matter that both lives into core.

jhodgdon’s picture

To paraphrase heyrocker recently on another issue:

On a project this size, having standards that involve human judgment doesn't work. (They just lead to bikeshedding.)

So, our options are:
a) Keep the standard currently in our naming standards documentation, which is "Acronyms must be all caps".
b) Revise it slightly, such as "Acronyms must be all caps. Don't put two acronyms in a row." or some other concrete suggestion(s) that would increase readability for specific circumstances.
c) Reverse the current standard, and say "Acronyms should also use CamelCase."
d) Not have a standard at all.

I think (d) is the only really terrible option. It will lead to code with a mix of camel and caps acronyms, which if nothing else is jarring (for instance, we could have the same acronym in one class in Camel and another in ALLCAPS being used in the same code), and/or bikesheds about which looks better for a given class or family of classes.

I think (b) is the hardest to do in a reasonable way, and the hardest to follow consistently. We should strive for clear standards that are easy to understand and enforce, and ideally that Coder and other automated testing scripts could check.

In which case, although I personally find it somewhat ... distasteful, let's say... I think probably (c) is the best standard, because it can easily be machine checked by Coder and (a) can't.

Anonymous’s picture

+1 for (c).

pounard’s picture

Very good point, +1 for c too.

neclimdul’s picture

c)

threewestwinds’s picture

c), for the subjective opinion that it leads to more readable names. XmlEncoder > XMLEncoder. CamelCase is usually nice because the capitals visually break names up into chunks.

merlinofchaos’s picture

Just so it's on record: I do not agree that turning acronyms into words so they match camel case is 'more readable' and reject that particular argument. (I have no input on the other arguments, just that particular one, but I still support option b)

tim.plunkett’s picture

ViewsUi is soooo painful to look at, but it is required for the naming of ViewsUiBundle anyway thanks to Symfony\Component\DependencyInjection\Container::camelize(), so I guess c) is the only real option.

jhodgdon’s picture

I agree with #64/65 that camel case acronyms are yucky to the max. I also don't like option (b) until we can articulate what exactly the terms of (b) would be. It looks like (#65) there are also cases in core where we cannot use ALLCAPS... which again leads to (c) as being the main viable option, even if we have to hold our noses when we look at things like ViewsUi and ClassContainingHtmlInTheName and XmlLoader and such stuff.

webchick’s picture

Status: Needs work » Reviewed & tested by the community

I really don't like abusing the critical status like this; we have more than enough tasks that actually block release.

However, it seems like C) is our only option, so RTBC.

tstoeckler’s picture

Status: Reviewed & tested by the community » Needs review

After the current standard of using CAPS in class names was established after a majority vote of a large number of core developers, it seems this is a bit quick to revert that decision. I think the fact that we currently use Symfony's camelize() and, thus, are forced to use ViewsUi instead of ViewsUI is unfortunate, but 1) certainly avoidable, although probably not cleanly, and 2) I'm pretty sure that was already clear when the original decision was made.

Although I'm personally for using CAPS for acronyms, my point is rather a procedural one here. I'm totally fine if we end up with a consensus on using CamelCase for acronyms, I just don't have the impression that we have such a consensus yet.

Setting back to "needs review".

webchick’s picture

Priority: Critical » Normal

I don't see a way of refuting #65 nor the last paragraph of #59, and certainly don't want to introduce hackish workarounds in our code base just to allow for an arbitrary standard that a lot of us happen to prefer (I too prefer ALL CAPS since they are, in fact, acronyms, but these are clear-cut technical arguments in my view, which is nice because preferences are by nature arbitrary and we should not be deciding coding standards via democracy of the handful of people who happened to stumble across random issues. :P~~~).

Moving this back down to normal. Sorry, Jennifer. If you feel strongly, let's split the difference at major but this is only blocking a "normal" priority initiative.

jhodgdon’s picture

OK on the issue status.

So. It does seem to me that most of us who came down on the side of ALLCAPS have switched to CamelCase for pragmatic reasons, and expressed a willingness to hold our noses at the ugliness (in our eyes) of it all.

Let's leave this at RTBC for 3-5 days and see if anyone else strongly objects. I will update the issue summary with our current plan. (As a note, setting policy/standards issues to RTBC seems to be the only way to attract people to make last-minute objections or forever hold their peace etc.)

I'm going to update the summary then change status...

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

I have updated the issue summary based on our current proposal.

If you think option (c) (CamelCase) is unacceptable, please speak up now. We'll adopt it in 3-5 days if there are not good reasons for doing otherwise.

Note that some people are going to think either (a) or (c) is "ugly"... I fall into the "c is ugly" camp myself. However, given that there are problems with (a) (UPPERCASE) and good reasons to want to have a clear concise enforceable standard (so far eliminating (b) and (d)), I am willing to hold my nose and say we should adopt (c). So... If you have a reason other than "I personally find it ugly" to decide against (c), please speak up. Given that some people find (a) ugly too, I don't think personal preference for elegance is going to hold sway as an argument here.

tstoeckler’s picture

It does seem to me that most of us who came down on the side of ALLCAPS have switched to CamelCase for pragmatic reasons

All right, that wasn't clear to me, and to be quite honest still isn't, but I digress...

I wanted to point out, however, that this is not only a matter of "ugly" but also a matter of consistency. CamelCase is consistent with Symfony and PHP-FIG, while CAPS is consistent PHP core. So consistency itself cannot decide this for us, but I wanted to point out that there are substantial and non-subjective arguments to be made for either side.

sun’s picture

  1. There are multiple possible incarnations, preferences, and interpretations here.
  2. PHP itself gives a shit about case-sensitivity of class names.
  3. So the saddest thing here is that we're only forced to make a hard decision due to PSR-0. Without that inanely bogus aspect of PSR-0, we wouldn't have to care, at all, and could simply mark this won't fix.
  4. Rewriting and overriding DepedencyInjection::camelize() and DependencyInjection::underscore() into implementations that care for proper capitalization of acronyms would not only be a mind-boggling and resource-intensive monster of a PCRE, but would also inherently break class-loading for external classes — again, thanks to PSR-0.
  5. Without doing so, some/certain class names in our code base would have to follow Drupal's coding standards, whereas all class names that may be derived via DependencyInjection::camelize() cannot, due to technical constraints.
  6. Therefore, the only possible solution here is indeed c). And AFAICS, there's no point in discussing this further.

The only hope is that there will be a PSR-0b standard at some point. (And apparently, php-fig did not think about a proper versioning scheme for the individual standards...)

Wim Leers’s picture

"c is ugly" +1

I think #59, #65, #71 and #73 are sound arguments — and note that the authors of those comments all think "c is ugly". So we're in this fun position where everybody hates c, but we have to use it anyway. Thanks to PSR-0, as #73 explained so nicely.


However, I wonder if we can cheat? Like #65's ViewsUiBundle, I ran into CKEditorBundle.php not working, so I had to rename it to CkeditorBundle.php. But… I realized just now that I'm in fact using c (CamelCase) for the filename, but a for the class name! I.e. CkeditorBundle.php contains CKEditorBundle (simply because I forgot to also rename the class). Only ugliness for filenames trumps ugliness everywhere in the codebase, but I guess this approach could cause trouble… :)

Crell’s picture

To be more specific, the "PSR-0 problem" is that the class name must match the file name... which is on its face not a problem, except that PHP is case-insensitive for class, function, and method names but *some* file systems are case-sensitive (anything on *nix family OSes, which means anything but Windows), which therefore means class names become effectively case-sensitive for autoloading purposes.

A lot has been made about that and the "problems" it can cause, but it's only a problem if your code base is still treating class names as case insensitive in the first place. I don't actually know anyone who does that (at least not anyone that I wouldn't object to hiring), and the only place we have a case-insensitive method name in all of Drupal is, AFAIK, in ViewSubscriber, which we hope to get rid of eventually anyway.

Just providing some background here. I'm OK with option (c) (XmlThingie, not XMLThingie). I could also live with (c) with occasional exceptions, since as long as the file name and class name in the code are consistent there's no runtime issue.

RobLoach’s picture

JsonldRdfSchemaNormalizer, not JSONLDRDFSchemaNormalizer.

We camel-case acronyms not because it looks prettier/uglier, but because it lets you know what the acronyms are. It's a readability decision, not a stylistic one. This is why C is the way to go.

[EDIT] PSR-0 has nothing to do with camel-casing. PSR-0 just maps a namespace to a path to make it easy to swap class loaders, whether it's camel-cased or not doesn't matter (as long as they're the same in both code and file system). That would be PSR-1 Basic Coding Standards.

[EDIT] #74 by Wim Leers:

However, I wonder if we can cheat? Like #65's ViewsUiBundle, I ran into CKEditorBundle.php not working, so I had to rename it to CkeditorBundle.php. But… I realized just now that I'm in fact using c (CamelCase) for the filename, but a for the class name! I.e. CkeditorBundle.php contains CKEditorBundle (simply because I forgot to also rename the class). Only ugliness for filenames trumps ugliness everywhere in the codebase, but I guess this approach could cause trouble… :)

As long as both the file and the class is referenced with the same case, then you'll be fine. The class loader doesn't change the case on you in any way. If it's CkeditorBundle.php, then call the class "CkeditorBundle".

neclimdul’s picture

I also have been struggling to find any reason PSR-0 has anything to do with this. Lets not kid ourselves, we are using a lot of classes in this release. Therefore as Drupalers that love standards we have to agree on one to organize our efforts. Even if they where all in 1 giant file this would still be a thing because of the number of classes and that's what the discussion has been about.

neclimdul’s picture

As a note about the CamelCase'ing, I like C because it has fewer exceptions and judgments involved. I don't have to rearrange or fudge or generally expend any brain power on a variable name who has logically sequential acronyms they just work and I can move on to solving the problem at hand. Both B and C have short falls, I just find C's simpler and easier to deal with.

pounard’s picture

I thought in the past that C was ugly, but in the end we get used to it, and it's not. It's just a matter of standard. +1 to #76, C solution actually make it more readable in cases like this. I agree that SomethingUi is unelegant but we can live with it. As Crell said, I'm OK with occasional exceptions too.

merlinofchaos’s picture

Just as a note: Our use of ::camelize() is limited to changing module names (i.e, views_ui) into something suitable for class naming (i.,e ViewsUi) for auto-detection purposes.

We could work around this by having modules define those strings in their .info file equivalents if we really wanted. I'm not saying we SHOULD, but I am saying that we can remove that as a technical limitation. It does mean that we can reduce the weight that technical argument applies, since we could solve it by asking the module what its camelcased name should be, and if it's something like views_ui it can say "I should be ViewsUI" in some manner, and if it doesn't have an answer, camelcase() can then handle the default case. Which happens to work adequately for most things. Just not *UI.

I agree with neclimdul; I don't see how PSR-0 cares about camelization. File names have to exact-case match class names, but that's irrelevant to whether or not it's XmlMangler or XMLMangler.

As for Rob Loach's argument, it looks like this:

Readability is (for many people) improved if you have a single acronym. i.e, XMLMangler is easier to parse, for me at least (and for many others) than XmlMangler. From comments, this is clearly not true for some, so the mileage of this argument varies and is subject to opinion.

Readibility suffers if you have multiple acronyms together.

XMLRDFMangler is less readable than XmlRdfMangler. Maybe. In RobLoach's example above, he actually misread it since it's JSON-LD not JSON ID :) -- so I'm not sure whether he successfully made his point or worked around it. Either way, that class name isn't readable as is, but that's partly because lowercase l and uppercase I are indistinguishable in some fonts.

RobLoach’s picture

XMLRDFMangler is less readable than XmlRdfMangler. Maybe. In RobLoach's example above, he actually misread it since it's JSON-LD not JSON ID :) -- so I'm not sure whether he successfully made his point or worked around it. Either way, that class name isn't readable as is, but that's partly because lowercase l and uppercase I are indistinguishable in some fonts.

Haha, proof that this readability change would help :-) . JsonLdRdfSchemaNormalizer does make more sense.

merlinofchaos’s picture

Right, except actually it's all one acronym: thus, Jsonld not JsonLd

neclimdul’s picture

The title of json-ld.org(I went and looked this up because I didn't know) implies that's not the case though. "JSON for Linking Data." This concludes your pointless off-topic comment.

catch’s picture

Title: [policy, then patch] Case of acronyms in class names (SomethingXSSClassName versus SomethingXssClassName) » Patch for: Case of acronyms in class names (SomethingXSSClassName versus SomethingXssClassName)
Status: Reviewed & tested by the community » Active

I really dislike how (c) looks but agree with all the comments supporting it. Let's do a patch.

jhodgdon’s picture

Did someone update the standards documentation then?

jhodgdon’s picture

Quick docs fix: I updated http://drupal.org/node/608152 just now to reverse the previous standard that was there. Do we need to update any other documentation or examples?

jhodgdon’s picture

Issue summary: View changes

Update to current proposal (CamelCase) and summary of options

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

anoopjohn’s picture

Issue summary: View changes

Ran into this issue while trying to figure out how I should case a function with an acronym in it. Thought I should perhaps bump this up in the list and take this issue to a closure.

The documentation on the standards seem to have been updated though - https://www.drupal.org/node/608152

Updated issue summary with an Edit section with the decision

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

quietone’s picture

Version: 8.9.x-dev » 9.3.x-dev
Status: Active » Closed (outdated)

As pointed out in #93 the standard for naming has been decided and is documented at naming conventions. The class names in core have been updated, making the patch here outdated. And since coding standard fixes are now done by sniff this can be closed as outdated. See #2571965: [meta] Fix PHP coding standards in core

If this is incorrect reopen the issue, by setting the status to 'Active', and add a comment explaining what still needs to be done.

Thanks!