-
Notifications
You must be signed in to change notification settings - Fork 26.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added naming conventions for UPPERCASE use cases. #255
Conversation
I'm ok with the uppercase for constants but not the namespace or global variables bit. Namespaces should be whatever case you're exporting (constructors in Pascal, functions in camel, etc), and there shouldn't be global variables :) |
Agree than namespacing in general can be any preferred casing, but nested object namespacing is slightly different. think YAHOO.util.Dom.myMethodName. When you're working with a large application, pulling in 20+ possible namespaces, generally you want a parent or overarching namespace followed by the section or app specific namespace, creating a nested object namespace. The fix is specifically related to nested object namespacing, not general namespacing, if that makes sense. Also agree that there shouldn't be globals, other than the nested object namespace or single namespace, but if a developer is creating global, it should be identified in some manner. Most guidelines i've seen have used uppercase for this. That said, should I create a new pull request to get the contant use case added? |
Ok cool makes sense, we can keep this together. Some comments on the PR otherwise this is 👍 |
README.md
Outdated
@@ -1161,8 +1160,34 @@ | |||
this.firstName_ = 'Panda'; | |||
|
|||
// good | |||
this._firstName = 'Panda'; | |||
this._firstName = 'Panda'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove trailing whitespace
Status of this? |
I would be happy to merge this after a rebase on the ES6 styleguide. |
I was just looking for a style guide rule about constant naming and found this open issue. I was having doubs whether we would still have a need for UPPERCASE_CONSTANTS_LIKE_THIS when we have const ActionTypes = {
theFirstActionName: 'theFirstActionName',
theSecondActionName: 'theSecondActionName',
}; if If you would use the uppercase naming, you would probably only do the property names uppercase and leave While Googleing some more about this, I found this page about the Google's C++ style guide has another variation: camel case prepended with a 'k' (https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Constant_Names). Personally though, I don't see why it can't just be camel case without the 'k'. In any case I think it's good to make this more explicit in the JavaScript style guide. So how about just a small section under 'variables' that clarifies |
|
Right you are. Subtle but important difference. Sounds like a good basis for explaining more explicitly when or when not to use uppercase. Below is an idea for a new section under 'naming conventions'. I can make a PR later if you'd like, but perhaps let's iterate on the text here first? I used the api key example used earlier in this PR. Wondering if this is indeed a correct example for this use case? To me it's still a bit vague what "value that won't be changed" means exactly. When I read your comment I realized that it's about changing the initial assignment instead of reassigning it later. But an API key can change, right? 22.9: Use SCREAMING_SNAKE_CASE for values that shouldn't be changed.
|
@maartenverbaarschot changed programmatically - clearly anything typed into code can be edited :-) The distinction is less important for strings, since they're immutable, but |
If I use If we took it back a step and said, "only for values that you intend never to change" - similar to what @maartenverbaarschot was suggesting - are there strict rules one could follow to determine that? |
@treshugart I've found this to be a bright enough line (in order): const OUTSIDE_VOICE = 'unchanging parameter';
const insideVoice = new NamedConcept(); These are constant values (i.e. the common meaning of constant) as opposed to |
whats the status here? |
Method and variable names should be camelCase, but what about names that are abbreviations and are usually spelled in ALLCAPS? The Date object preserves the ALL CAPS within method names like |
@dandv that is badly named for a number of reasons - including the one you identified. It also isn't limited to just requesting XML. |
Initialisms like HTTP, XML, USA, JSON, etc should always remain capitalized (or be entirely lowercased), and never CamelCased or PascalCased. |
@justjake If someone else picks this PR up and does the rebase, does the merging offer stand? I'd be happy to do so. |
@ljharb Great :). I'm happier w/ that solution too, but thought this had gotten dusty. I see @estelle's last activity in this thread as May 2015 (#255 (comment)). |
My interpretation of the ideas here is that: UPPERCASE_VARIABLES are letting the programmer know that they can trust the variable (and its properties) not to change This is most helpful in situations where the programmer would be unsure if the variable might ever change. There are a few cases that have come up in the above comments:
Which leads to a rule of: You may optionally UPPERCASE a constant only if it (1) is exported, (2) is a const, and (3) the programmer can trust it (and its nested properties) to never change. Examples: // bad
const PRIVATE_VARIABLE = 'should not be unnecessarily uppercased within a file';
// bad
export const THING_TO_BE_CHANGED = 'should obviously not be uppercased';
// bad
export let REASSIGNABLE_VARIABLE = 'do not use let with uppercase variables';
// ---
// allowed but does not supply semantic value
export const apiKey = 'SOMEKEY';
// better in most cases
export const API_KEY = 'SOMEKEY';
// ---
// bad - unnecessarily uppercases key while adding no semantic value
export const MAPPING = {
KEY: 'value'
};
// good
export const MAPPING = {
key: 'value'
}; This seems consistent, helpful, and not overbearing. Thoughts? Any cases I'm missing? |
@shanemileham: nice writeup! How about members of an options/parameters object that are not intended to be changed during the program's execution, but are configurable nevertheless?
|
@dandv separate from that being invalid syntax, if you want object properties to be constant, it's best to actually make them nonconfigurable and nonwritable. @shanemileham that does seem like a good writeup. @estelle, are you willing to either update this PR or check the "allow edits" box? |
@ljharb edits allowed |
@shanemileham would you mind converting your comment into a commit on your own fork, and linking that commit here? I can cherry-pick it into this PR (which I've just rebased). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM pending one comment
README.md
Outdated
@@ -3139,6 +3139,44 @@ Other Style Guides | |||
]; | |||
``` | |||
|
|||
<a name="naming--uppercase"></a> | |||
- [23.10](#naming--uppercase) You may optionally uppercase a constant only if it (1) is exported, (2) is a const, and (3) the programmer can trust it (and its nested properties) to never change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shanemileham "is a const" isn't necessarily clear to me; a const in JS is a variable that's never reassigned. Is that what you mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Literally a const
and not let
. This just asserts that in addition to not being reassigned, it can't be reassigned. What is the best way to make this more clear?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ljharb (tagging you since idk if you get a notification otherwise)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha - I'll add backticks around it, but otherwise that's probably good as-is :-)
Per 23.6, Does this mean that if I have: const LANGUAGES = [
// ...
];
export default LANGUAGES; I should name it And do: import LANGUAGES from './LANGUAGES'; |
@teohhanhui Great question! Short answer is yes imho. 23.6 must be followed in all cases; uppercasing is simply optional (superseding 23.8 just as it supersedes normal case rules for constants), and if you choose to do so on a default export, that seems to be the logical conclusion. It seems weird at first, but it's actually really nice to know that an entire file is a constant with just a glance. Feel free to add that to this PR (assuming consensus) - I think people could benefit from seeing that explicitly. |
Hello everyone, A friend and I were questioning the uppercase convention that applies for some kind of constants. After a bit of digging, I found Google's take on this and MDN's take on this, but neither of those really answer the "why" of this convention. Thanks to everyone here, Airbnb has a "why" in its style guide :
Could someone give an example in which "the programmer" would treat a const differently in the code if it's an uppercased one vs a camel-cased one? Does knowing that a variable won't or will change affects what the programmer will do? If there is no difference, why would one want to keep using this convention? Thank you! |
@borisghidaglia there's a difference between a |
Hello @ljharb, thanks for your answer. Of course I agree that these two cases are different. In my question above, I asked for "an example in which "the programmer" would treat a const (be it a constant reference or a constant value) differently in the code if it's an uppercased one vs a camel-cased one". In my mind, the programmer will have to check the actual value of the const anyway:
Until recently, I was following this rule "by default", and at some point I found myself suggesting to uppercase a const name to a friend during a review. He asked why and I said it was a convention to indicate constant values. He then asked what would change in my usage of this const if it was uppercased vs if it was not... and I could not answer! I mean, one could argue that the programmer can "assume" that it is a constant value and "guess" its value, thus use the const without even checking its content... but this seems unlikely to me. If you had a practical example to share, it would be really helpful to understand when this convention is really useful. Thanks again! |
@borisghidaglia because it's the signal used in nearly every programming language since the invention of computers to indicate a value that will not change. That someone might still have to check doesn't change anything; it's still valuable after they check to convey intent. |
@ljharb What this signal adds? From a practical point of view, it seems to me it doesn't change anything: it won't change the time I will spend understanding the code or writing it. If we uppercase to add intent the constant won't change, why not lowercasing (e.g. my_variable) to add intent the constant will change? This adds intent too but we don't do it. So why the signal the constant won't change is important and merit to be marked? |
There is NOTHING more valuable - in a practical sense or any other - than improving readability, which better conveying intent always does. Most values change, which is why a constant value needs special annotation - because it's rare. |
That's what I assumed when I did my research before posting here. But even though history of computers is interesting, I don't feel we can use that fact alone to justify this practice.
I think you are right on this one. It was a bad argument. As a friend told me afterwards: "it's not because as programmers we often need to check what's inside a variable that we should name them randomly". And in fact properly naming a variable conveys intent as well.
I also agree. But improving readability is not the end goal by itself. Improving readability matters because it allows the programmer to grasp the code quicker, and to grasp it correctly. And that is the problem: I have a hard time finding an example where it is cristal clear that uppercasing makes the difference in these regards. To try to answer myself to the need for an example I had above, I came up with these examples, stemming from my friend's example about random variable names:
But we could argue that the name "delay" was too unclear to begin with, and that if we used a proper name, we would not really need uppercasing. For me the conclusion to this is that I was too strict about this convention, thinking it was really making a difference, when in fact it seems to be a much more incremental improvement than properly naming variables (for example). That is probably why this convention is optional in Airbnb style guide! Thanks @ljharb for your time, I appreciate it! |
Constants, globals and nested object namespacing, by convention, use all UPPERCASE for those variable names.
also removed an extra line space from line 142.