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
- Agree upon a standard.
- Document it.
- 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
Comment | File | Size | Author |
---|---|---|---|
#29 | drupal-1627350-29.patch | 98.2 KB | tim.plunkett |
#25 | acronym-case-1627350-22.patch | 98.2 KB | xjm |
#25 | interdiff.txt | 8.07 KB | xjm |
#19 | drupal-1627350-19.patch | 92.45 KB | tim.plunkett |
#18 | drupal-1627350-18.patch | 57.69 KB | tim.plunkett |
Comments
Comment #1
aspilicious CreditAttribution: aspilicious commented.
Comment #2
merlinofchaos CreditAttribution: merlinofchaos commentedJust 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.
Comment #3
Tor Arne Thune CreditAttribution: Tor Arne Thune commentedIn 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.
Comment #4
Tor Arne Thune CreditAttribution: Tor Arne Thune commentedAlso, 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? ;)).
Comment #5
RobLoachAcronyms 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:
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()
, notnew 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.
Comment #6
Tor Arne Thune CreditAttribution: Tor Arne Thune commentedJust 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.
Comment #7
merlinofchaos CreditAttribution: merlinofchaos commentedI 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.
Comment #8
jhodgdon+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.
Comment #9
aspilicious CreditAttribution: aspilicious commentedPatch 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...
Comment #10
aspilicious CreditAttribution: aspilicious commentedIs that correct? I have the feeling my file renames aren't processed...
EDIT: yeah file renames are missing
Comment #11
RobLoachComment #12
aspilicious CreditAttribution: aspilicious commentedWe probably also need a patch with the opposite approach. To compare those...
Comment #13
tim.plunkettI'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...
`extends Ftp` then, I guess
Wouldn't this be OpenIdTestBase, here and elsewhere throughout
Comment #14
sunI 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.
Comment #15
jhodgdon#14 ==> +1 (for once I agree completely with sun.) :)
Comment #16
xjmI find #14 compelling as well.
Comment #17
tim.plunkettI 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:
Also, shouldn't this be extended to methods as well?
Comment #18
tim.plunkettOkay, got part way done.
ApiCrudEfqFtpGdIpJsJsonLobMimeOpmlPeclPhpRdfRpcRssUiXmlComment #19
tim.plunkettBecause 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.
Comment #20
aspilicious CreditAttribution: aspilicious commentedStill 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
Comment #21
xjmSo 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.
Comment #22
tim.plunkettDuh I forgot uuid!
And yes, I had to do the two step process.
Comment #23
Tor Arne Thune CreditAttribution: Tor Arne Thune commentedAlso, 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...
Comment #24
tim.plunkettI saw that and decided abbreviation != acronym and left it
Comment #25
xjmEdit: 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.
Comment #26
Tor Arne Thune CreditAttribution: Tor Arne Thune commented@#24: I agree, let's not go down that road.
Comment #27
xjmYeah, they are Templates, not TemPLates or TastilyPreparedLattes, so that one should stay as-is.
Comment #28
sunCOM 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...
Comment #29
tim.plunkettRerolled for Com/COM.
Reminder, this patch cannot be applied on OS X!
Comment #30
jhodgdonTwo 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...)
Comment #31
xjmRegarding #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.Comment #32
jhodgdonRegarding 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?
Comment #33
aspilicious CreditAttribution: aspilicious commentedI *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. :)
Comment #34
jhodgdonRE #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).
Comment #35
Crell CreditAttribution: Crell commentedExcept 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?
Comment #36
tim.plunkett@Crell, I updated the "Proposed resolution" in the summary.
Comment #37
sunPerhaps 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).
Comment #37.0
sunClarify the proposed resolution
Comment #38
tim.plunkettDuh, I wrote exactly the wrong thing. Fixed.
Comment #39
Crell CreditAttribution: Crell commentedHm. "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.
Comment #40
merlinofchaos CreditAttribution: merlinofchaos commentedI 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.
Comment #41
jhodgdonLet'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?
Comment #42
catchI 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.
Comment #43
jhodgdonI 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.
Comment #44
sunPR 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)
Comment #45
neclimdulI 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.
Comment #46
Crell CreditAttribution: Crell commentedJust 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.
Comment #47
Anonymous (not verified) CreditAttribution: Anonymous commentedWhen 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.
Comment #48
jhodgdonAnother possibility for acronyms with acronyms is to alternate case, such as JSONldEntityNormalizer.
Comment #49
Crell CreditAttribution: Crell commentedjhodgdon: That would be even more confusing and inconsistent with the entire rest of the PHP universe, as well as itself.
Comment #50
quicksketchI'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
orelement.innerHTML
Interestingly, "Id" is not all uppercase, I'm guessing because it's really an abbreviation rather than an acronym. i.e.
document.getElementById()
.Comment #50.0
quicksketchBrain fart.
Comment #51
Crell CreditAttribution: Crell commentedPHP 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.
Comment #52
quicksketchIn 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.
Comment #53
Crell CreditAttribution: Crell commentedquicksketch: Have any evidence of that? The PHP-FIG group has recently standardized on SomethingXssClassName for all of its code.
Comment #54
quicksketchAfter 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.
Comment #55
Wim LeersALL 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".
Comment #56
Crell CreditAttribution: Crell commentedI 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)?
Comment #57
jhodgdonWe 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
Comment #58
pounardI think it doesn't really matter that both lives into core.
Comment #59
jhodgdonTo 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.
Comment #60
Anonymous (not verified) CreditAttribution: Anonymous commented+1 for (c).
Comment #61
pounardVery good point, +1 for c too.
Comment #62
neclimdulc)
Comment #63
threewestwinds CreditAttribution: threewestwinds commentedc), 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.
Comment #64
merlinofchaos CreditAttribution: merlinofchaos commentedJust 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)
Comment #65
tim.plunkettViewsUi 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.
Comment #66
jhodgdonI 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.
Comment #67
webchickI 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.
Comment #68
tstoecklerAfter 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".
Comment #69
webchickI 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.
Comment #70
jhodgdonOK 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...
Comment #71
jhodgdonI 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.
Comment #72
tstoecklerAll 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.
Comment #73
sunDepedencyInjection::camelize()
andDependencyInjection::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.DependencyInjection::camelize()
cannot, due to technical constraints.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...)
Comment #74
Wim Leers"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 intoCKEditorBundle.php
not working, so I had to rename it toCkeditorBundle.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
containsCKEditorBundle
(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… :)Comment #75
Crell CreditAttribution: Crell commentedTo 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.
Comment #76
RobLoachJsonldRdfSchemaNormalizer, 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:
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".
Comment #77
neclimdulI 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.
Comment #78
neclimdulAs 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.
Comment #79
pounardI 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.
Comment #80
merlinofchaos CreditAttribution: merlinofchaos commentedJust 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.
Comment #81
RobLoachHaha, proof that this readability change would help :-) . JsonLdRdfSchemaNormalizer does make more sense.
Comment #82
merlinofchaos CreditAttribution: merlinofchaos commentedRight, except actually it's all one acronym: thus, Jsonld not JsonLd
Comment #83
neclimdulThe 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.
Comment #84
catchI really dislike how (c) looks but agree with all the comments supporting it. Let's do a patch.
Comment #85
jhodgdonDid someone update the standards documentation then?
Comment #86
jhodgdonQuick 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?
Comment #86.0
jhodgdonUpdate to current proposal (CamelCase) and summary of options
Comment #93
anoopjohn CreditAttribution: anoopjohn at Zyxware Technologies commentedRan 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
Comment #96
quietone CreditAttribution: quietone as a volunteer commentedAs 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!