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

Types and variables naming #12

Closed
mlewand opened this issue Nov 5, 2014 · 18 comments
Closed

Types and variables naming #12

mlewand opened this issue Nov 5, 2014 · 18 comments
Assignees

Comments

@mlewand
Copy link

mlewand commented Nov 5, 2014

Overview

We should conclude the way that we want to name types / variables.

Common rules

There is no doubt that we want to use camelCase, that's cool. There are few details that needs to be discussed:

Acronyms

How to use acronyms? Do we use them like a word, or all uppercased?

Just off the top of my head, I can name one monster: XMLHttpRequest.

CKE4: Generally we're doing really good job treating acronyms as a words, I had really to look deep down to find some examples of violating such rule.

So we commonly have members like: getHtml, getInnerXml, addContentsCss.

Violating members are: onCDATA, fromBBCode.

I think that we should treat all acronyms (no matter what) as a word.

Variables

Camelcase, starting with lowercase, eg. addItem, requestType, getCount, select.

Types

The same rules applies as to Variables, with the exception that they're uppercased, so:

Editor, ContentFilter, FocusManager, MenuButton, Command, XmlParser

Namespaces

How do we want to tread namespaces? Do we want to name them as variables, or like types?

Naming it as variables would make sense, so it would make chains like:

dom.Element, ui.button.Handler, ui.dialog.JumpingDialog

How do we name main CKEDITOR global variable?

I believe for compatibility sake we should stick to capitalized? So CKEDITOR remains.

I ask politely for making a conclusion as I want to apply our new standard to the AC right away.

@fredck
Copy link
Contributor

fredck commented Nov 5, 2014

Acronyms: for me it is all about readability: fromBbcode is damn weird... BBCode is part-acronym, part word...fromBbCode would be weird as well... just like the recent UI issue: e.g. ButtonUI or ButtonUi.

As I said, MS has a standard for acronyms: if 2 letters acronym only, then both in the same case. So we have UI, BBCode (BB + Code), CData (C + Data), but Html, Xml, Wysiwyg. Sounds good for me.

Variables: agree - lowerCamelCase

Types (Classes): agree - UpperCamelCase

Internal namespaces: lowerCamelCase

Root global namespaces: ALLCAPS

@Reinmar
Copy link
Member

Reinmar commented Nov 5, 2014

As I said, MS has a standard for acronyms: if 2 letters acronym only, then both in the same case. So we have UI, BBCode (BB + Code), CData (C + Data), but Html, Xml, Wysiwyg. Sounds good for me.

+1.

  • Variables, method names, property names, function names - lowerCamelCase
  • Classes (Constructors) - UpperCamelCase
  • Middle part of the namespaces - lowerCamelCase (CKEDITOR.fooBar.booBar.Constructor)
  • Root - ALLCAPS
  • All tokens which don't belong to any of the above groups (button names, plugin names, etc.) - lowerCamelCase
  • Module names - I'm hesitating between:
    • always lowerCamelCase,
    • lowerCamelCase if represents a namespace or UpperCamelCase when module exposes only a constructor.
  • Files and directories names - I'm hesitating between:
    • follow the convention of the related name or lowerCamelCase. So directories that represents modules or fragment of namespace - lowerCamelCase. Files which contains a single constructor - UpperCamelCase,
    • always lowerCamelCase (will be much easier).

@mlewand
Copy link
Author

mlewand commented Nov 5, 2014

@fredck

I see your point. I'm fine with such approach.

@Reinmar

All tokens which don't belong to any of the above groups (button names, plugin names, etc.) - lowerCamelCase

This is actually good point, that did not get attention.

lowerCamelCase if represents a namespace or UpperCamelCase when module exposes only a constructor.

I would go with this approach for module names.

Files and directories names - I'm hesitating between

That should be exactly the same as for module names. No matter what decission is taken.

@Reinmar
Copy link
Member

Reinmar commented Nov 14, 2014

I've been just thinking about test files names. I've got a file in v4 which is called focusaftersettingdata.md. Unlike most file names this is a sentence, not a token from code. I think that it would be actually reasonable to have a different naming convention for such files, because it was very intuitive for me to call it focus-after-setting-data.md. I think that focusAfterSettingData.md looks in this case very odd. WDYT?

@Reinmar
Copy link
Member

Reinmar commented Nov 14, 2014

On the second thought... maybe focusAfterSettingData.md isn't that bad :D. Still - I'm curious what's your opinion.

@fredck
Copy link
Contributor

fredck commented Nov 14, 2014

As for file/directory names, we're not any more talking about code. We must have a fit all solution because files/directories can have any kind of content.

Btw, we're blocking this this issue uselessly. The convention for file names will not in any way impact in the convention for in-code names. We have already issue #3 for that purpose so we should not discuss it here.

@fredck
Copy link
Contributor

fredck commented Nov 14, 2014

I think we reached a common agreement when it comes to the name conventions in code. Do you guys see any pending topic?

@Reinmar
Copy link
Member

Reinmar commented Nov 14, 2014

I think that there's still a lot of details that we need to clarify but they will come out with time. E.g. whether we use underscores to mark protected/private stuff, whether we use plural or singular names for stuff like the "smiley" plugin? Do we always create getFoo methods, or can we sometimes create just foo, Do we shorten some things like "attributes" to "attrs"? Keystrokes or hotkeys? :D Etc.

We agreed about the most important basics, but I would keep this ticket open to track other related decisions. I know that it's not necessarily the same subject as how to format names, but I feel that if we won't generalise this subject, we just won't have anything more written down later.

@fredck
Copy link
Contributor

fredck commented Nov 20, 2014

I've updated the documentation, aligning it to the stuff that we agree here so far:
https://github.com/ckeditor/v5-protos/wiki/Code-Style#naming

@gregpabian
Copy link

I'd add one more thing regarding module dependencies - when we're requiring multiple dependencies, then the module definition should look like this:

CKE.define( [
    'ckeditor',
    'tools',
    'mvc/model'
], function(
    CKE,
    tools,
    Model
) {
    'use strict';
    ...
} );

It looks cleaner and reads better than a huge one-liner.

@fredck
Copy link
Contributor

fredck commented Nov 20, 2014

I'd add one more thing regarding module dependencies...

I agree, but let's wait for opinions as this is one of the few weak points of AMD.

Other options:

  • One line up to 80 chars line length.
  • One line up to a number of dependencies (5). I like this one.
  • Always one line.

@gregpabian
Copy link

One line up to 80 chars line length.

I don't think limiting lines to 80 characters makes sense these days - nearly every screen is in 16:9(10) ratio, so I'd propose going with something bigger, 120 characters maybe?

@fredck
Copy link
Contributor

fredck commented Nov 20, 2014

I don't think limiting lines to 80 characters makes sense these days - nearly every screen is in 16:9(10) ratio, so I'd propose going with something bigger, 120 characters maybe?

80 chars is mainly because of console. I never use console for code stuff, so I'm definitely fine to go with 120 chars. Others?

@fredck
Copy link
Contributor

fredck commented Nov 20, 2014

I think that there's still a lot of details that we need to clarify but they will come out with time. E.g. whether we use underscores to mark protected/private stuff, whether we use plural or singular names for stuff like the "smiley" plugin? Do we always create getFoo methods, or can we sometimes create just foo, Do we shorten some things like "attributes" to "attrs"? Keystrokes or hotkeys? :D Etc.

We agreed about the most important basics, but I would keep this ticket open to track other related decisions. I know that it's not necessarily the same subject as how to format names, but I feel that if we won't generalise this subject, we just won't have anything more written down later.

I would like to start splitting issues, otherwise the discussions around them start getting too long and confusing. Therefore I've opened #15 and #16 for the remaining naming things.

@fredck
Copy link
Contributor

fredck commented Nov 20, 2014

I'd add one more thing regarding module dependencies...

I agree, but let's wait for opinions as this is one of the few weak points of AMD.

Other options:

One line up to 80 chars line length.
One line up to a number of dependencies (5). I like this one.
Always one line.

I've opened #18 for this.

@fredck
Copy link
Contributor

fredck commented Nov 20, 2014

I don't think limiting lines to 80 characters makes sense these days - nearly every screen is in 16:9(10) ratio, so I'd propose going with something bigger, 120 characters maybe?

80 chars is mainly because of console. I never use console for code stuff, so I'm definitely fine to go with 120 chars. Others?

I've opened #19 for this.

@fredck fredck self-assigned this Nov 20, 2014
@fredck fredck added the review? label Nov 20, 2014
@fredck
Copy link
Contributor

fredck commented Nov 20, 2014

Considering that missing topics have been moved to dedicated issues, I'm asking for review on the docs so we can close this one:
https://github.com/ckeditor/v5-protos/wiki/Code-Style#naming

@fredck
Copy link
Contributor

fredck commented Feb 18, 2015

As for for what has been discussed so far, I'm closing this one.

@fredck fredck closed this as completed Feb 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants