Skip to content
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

lowercase instance var in example code #2411

Merged
merged 1 commit into from Mar 20, 2013

Conversation

philfreo
Copy link
Contributor

No description provided.

caseywebdev added a commit that referenced this pull request Mar 20, 2013
lowercase instance var in example code
@caseywebdev caseywebdev merged commit 5c183b2 into jashkenas:master Mar 20, 2013
@jashkenas
Copy link
Owner

Revert this, please.

@caseywebdev
Copy link
Collaborator

Upper case instance variables?

@jashkenas
Copy link
Owner

Upper case global collections.

Aside: whatever kind of variable it is -- it certainly ain't an "instance" variable. That refers to a property of an instance of a class.

@tgriesser
Copy link
Collaborator

@jashkenas can you look at the discussion/changes in #2219 and comments at the base of #2412 regarding this -

I think the "upper cased collections" in the docs are misleading/confusing. Generally, upper cased variables are reserved for top level namespaces or constructor functions. Since there isn't really a concept of "global" in the docs, it is a bit confusing to not use the fairly standard naming convention to lowercase object instances (makes sense in the todo example where there is a bit more context, though I still think most just find it confusing).

If we revert this, we'll have to revert this in multiple places from the previous ticket as well.

@jashkenas
Copy link
Owner

Yes -- please revert it from both tickets.

If you go around and look at the codebases in many/most Backbone apps, you'll find that global namespaces and global collections are usually capitalized. It's a very nice convention to have, and something we should be keeping. Lowercased global variables are not a standard naming convention in any JavaScript I've ever seen -- I'd immediately assume that they were local.

@caseywebdev
Copy link
Collaborator

While I can't say I agree with this particular practice, I've reverted 😉

@braddunbar
Copy link
Collaborator

I don't feel strongly about how we do this in the docs but I also find the capitalized collection instances to be jarring.

In my experience, capitalized variable names are reserved for top level namespaces (e.g. window.Backbone, window.App) or constructor functions (e.g. Model, Collection, View). Seeing Documents in code I've never seen before, I assume/guess it's a constructor function or namespace. Further, I don't usually see window.documents, but instead window.App.documents in order to minimize the chance of a collision with third party libraries.

@tgriesser
Copy link
Collaborator

I agree with Brad, I guess it works okay if you're limited to a simple Todos app, but to put collection instances directly on the window is a pretty bad practice to encourage here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants