Quite a few issues were recently opened about coding standards and cleanup.
Let's agree on or bikeshed those things here, so we don't spam other issues with it.
Mostly we should follow Drupal 7 conventions.
However: The conventions for class names and class files in Drupal 7 are a joke. It says you should CamelCase your class, but typically there is no indication which module it belongs to. Also, there is no convention which file it should go into.
In Drupal 8 we get PSR-0, and one namespace per module (Drupal\\crumbs\\). In Crumbs we use the "PHP 5.2 compatibility solution" of xautoload: Classes are prefixed with the (lowercase/literal) module name. Then what follows is CamelCase fragments separated by further underscores.
At some point we should switch to PSR-0, but for now I think the existing convention has a decent DX and is much better than what other D7 modules do.
What I'm a bit undecided about is variable names, attribute names, function names, method names.
- functions: lower_case_with_underscore()
- "private" functions: _leading_underscore() ? Do these have a reason for existence? Usually I use this out of paranoia to avoid hook nameclashes.
- local vars in functions and methods: $lower_case_with_underscore
- static vars in functions: $lower_case_with_underscore. No leading underscore, as decided in another issue.
- public methods in classes: lowerCamelCase()
- public methods with suffix: lowerCamelCase_randomCaSED_suffix() (*)
- private and protected methods: lowerCamelCase() (no underscore)
- protected attribute: $this->lowerCamelCase
- class names: crumbs_MyClass_FurtherClassName_rAndomcASEDsuffix (*)
(*) "random cased suffix" - the point is, if we an already existing identifier becomes part of a class or method name, then we prefer to keep its exact spelling and case, rather than squeezing it into the convention. On the other hand, if the thing contains an underscore, and this would force us to go one directory deeper in /lib/, then we rather camelcase it.
Actually I am not 100% sure about this stuff, e.g. for
crumbs_InjectedAPI_hookCrumbsPlugins
I don't feel confident about this. In the past I used to say crumbs_InjectedAPI_hook_crumbs_plugins, but this would give us a dir structure of lib/InjectedAPI/hook/crumbs/plugins.
Do you (bojanz) agree with this?
I think there are still points in the code which don't follow those conventions.
Comments
Comment #1
bojanz commentedWhat you wrote is perfectly reasonable, and I have no problem with crumbs_InjectedAPI_hookCrumbsPlugins. It makes sense to me.
I use it to mean "other modules shouldn't ever use this, it's private and might change at any time". I find very few examples where this is needed in our codebase though, our helper functions are meant to be used by others if needed.
Comment #2
donquixote commentedLet's reopen as soon as we have something new to discuss.