Prefix all declared function parameter list identifiers with a

[Martii]

Applies to #262

[Zren]

Wait, we're now using hungarian notation?

[Martii]

Not familiar with that term but this will clear up coding issues especially in between object names and identifiers. Stands for argument btw... similar to our r requirement for regular expressions.

---

This is a perfect example right where I'm at too:

strat: aStrat.name,

vs. what we have been doing of:

strat: strat.name,

The latter can confuse anything.

[sizzlemctwizzle]

We're only prefixing function parameter names. Nothing else.
On Jul 14, 2014 4:09 PM, "Chris Holland" [email protected] wrote:

Wait, we're now using hungarian notation?

—
Reply to this email directly or view it on GitHub
#264 (comment)
.

[Martii]

We're only prefixing function parameter names. Nothing else.

Correct.

[Zren]

Oh, it's a strategy not array strategy. Hungarian notation prefixes the data type. Where a represents array.

In javascript, { key: value } key isn't executed. You have to use obj[keyName] to get/set using a variable. You could be more explicit about it by wrapping the key in quotes though. 'strat': strat.name

[Martii]

You have to use obj[keyName] to get/set using a variable

The main reason is for human readability... e.g. not having to figure out if it's a parm or an intermediate.

You could be more explicit about it by wrapping the key in quotes though.

That's probably something that could go into the STYLEGUIDE.md for sure.

[Zren]

That's probably something that could go into the STYLEGUIDE for sure.

If the key name is the same as a variable in the current scope or always?

[Zren]

This issue... has the sounds of the need to use an editor that you can set the color of parameters differently.

[Martii]

or always?

Always... if this goes in there. Let me finish with this issue and we can discuss that change in another one.

This... has the sounds of the need to use an editor that you can set the color of parameters differently.

I won't be distracted from this issue by debating what editor is better than another. Thanks though.

[Zren]

My point was that it makes it harder to read these variables, rather than easier. Using a different syntax colour for parameters is the least intrusive on the codebase. Too many variables are using shortforms that using this style will break the point of using the shortform and force to be renamed.

Point me to another javascript codebase that does this.

[Martii]

Your point is exactly to compare editor highlighting capabilities and that is off topic.

I've made my share of not catching STYLEGUIDE.md mistakes and you and sizzle have as well... part of the issue mentioned yesterday (that you didn't comment on but you did comment (albeit in the wrong issue ticket) ) ... is that the code is unreadable both human and logically. A simple a prefix requirement is inherently more readable and will discover other mistakes in our code... I think I found one a few days ago and don't feel like digging it up to cite... because this issue needs to get done.

[Zren]

but you did comment

Didn't read the entire thread, I only skimmed it. It was about removing old code and existing consistency. Not new styleguide standards or so I thought.

inherently more readable

...

harder to read these variables

[Martii]

Didn't read the entire thread.

That's something for you to improve upon. I reread every comment every day on current issues... sometimes even the old ones to see what has changed.

can no longer use shortforms in parameters.

In relation to what? What are you talking about? There is no logic removal with this.

I think you need to ask yourself why are you fighting a standard naming change that many projects already have in place. It's not that difficult to ignore the a unless it's needed to determine bugs... in this case it is. Every point you've brought up is moot... we all need to know if there's a global being passed versus and intermediate. Anyways you've put in your vague vote, albeit late... I'm finishing this up and not going to get into this further.

[Zren]

many projects already have in place.

Name 1

[Martii]

... improve overall consistency

 

Not new styleguide standards or so I thought.

 

Name 1

The entirety of Mozilla (a few thousand or so). I'm done having you argue incessantly on this... I am more than willing to accommodate reasonable change given there is sufficient proof... which you haven't given at all for this.

[cletusc]

Name 1

Greasemonkey, to name just one (for below, bolded is for arguments, italicized are points of interest):

Function and variable naming

Unless otherwise specified, use camelCasedNames.
Constants should be in UPPER_CASE.
Use InterCaps and capitalize the first letter of constructors.
Do not pollute the global namespace. Global (chrome window scoped) definitions should be avoided, but at least use a GM_ name prefix.
Global state variables should be prefixed with the letter g, e.g. gFormatToolbar.
Arguments (parameter names) should be prefixed with the letter a.
_Private members should start with _, e.g. internalFunction. They should not be accessed except by the class they are a member of.
Event handler functions should be prefixed with the word on, in particular try to use the names onLoad, onDialogAccept, onDialogCancel, etc., where this is unambiguous.
Function names, local variables, and object members have no prefix.
Try to declare local variables as near to their use as possible; initialize every variable.
Only declare a variable once in each scope.

Basically:

• Arguments start with a
• Privates start with _
• Globals start with g
• Event handlers start with on

It isn't System Hungarian notation, which is what you are thinking, but Apps Hungarian notation (System vs Apps Hungarian notation). Not everyone has a code editor that stylizes arguments different from other variables, nor should they be required. For example, Sublime doesn't know the difference between the two:

I am +0. Just quit spamming my inbox with petty arguments.

[Zren]

Apps Hungarian

Ah. The only ever project I've seen (system) hungarian is jQuery.DataTables, and they're moving away from it.

Proof

While our site is for userscript browser extensions like Greasemonkey, our site doesn't depend on it. They depend on us. The technologies we use don't use it.

• https://github.com/visionmedia/express/blob/master/lib/application.js#L206
• https://github.com/LearnBoost/mongoose/blob/3.8.x/lib/document.js#L402
• https://github.com/caolan/async/blob/master/lib/async.js#L118

Mozilla's style guide also has to deal with C++, and it makes sense for them to have a consistent style guide... I guess.

• Looks Ugly

aErr, aCb, aRes, aRep, aNext

Short forms are now capitilzed and they look terrible.

• Reads off as "a error" rather than "an error". Sure a = argument, but I shouldn't need to know that to read the code
• Most functions should be small enough for you to see the function definition. If not, you should know the name of the function and it's arguments anyways.
• Variable name that goes in != name defined for the argument.

Sublime doesn't know the difference between the two:

Your theme doesn't do it. It's possible to do as the scope is defined. Most themes don't do it as you don't normally need to treat arguments special.

[Martii]

Nearing completion.

A few areas are marked as Ambiguous because it is unclear on how to convert them because historically the STYLEGUIDE was not followed regarding unique identifier naming sequences. This is reason No. 1 for doing this... as a few of you haven't been paying attention to this much.

A few areas are also not in compliance with naming standards of not using $ ... some of those are notated.

Duplicate code for searching is historically present.

Have to run this through some final checks to see what is and isn't busted.

Known TODO:

• Unflag script returns 404
  • EDIT: OH MY this is present on current pro too and should be a separate issue.
• Still some aCb code in there... wasn't entirely sure if the nested async'd callbacks needed inner and outer designations.
  • EDIT: Going to do on separate issue.
• One parm of flags is unclear to migrate... notated on line notes and ES comment
• One parm of convert is unclear to migrate... notated on line notes and ES comment
  • EDIT: Not sure if this one is correct since original identifier was reused many times... can find by searching for Ambiguous in a comment.
• All sizzles points regarding POST instead of GETs and moving things around.
• Anything else missed.

---

@sizzlemctwizzle
Inconsistencies found:

• Non unique naming of identifiers
• Too simple of naming of identifiers sometimes.
• No clear method of adding options to any part of any of the code... sometimes it's declared locally (as options) other times it is passed (as aOptions now), and sometimes it's just there... this is vague.
• Usage of restricted characters such as $ for identifiers
  • UPDATE: see Martii@af46e15#commitcomment-7019444 and forward.
• Quite a bit of duplicated code... possibly dead code too... huge blocks of commented out code.
• I saw a couple, as in very few, intermediates to abstract the function parms... this is probably okay but not consistent across the board. Notated some of them with // Intermediates.
• Some future strict mode conflicts with inline functions not near the top of their parent function object. Not an issue right now but could throw an exception during future development. Didn't fix most of them. Needed for symmetry though especially for newer contributors.
• Possible globals found.
  • UPDATE: See Martii@af46e15#commitcomment-7019459 and forward.
  • UPDATE: See Martii@df94b62#commitcomment-7023970 and forward
• Inconsistent naming of global identifiers between modules when using require()

@cletusc
Sorry for the noise. Thanks for linking/posting the GM guidelines. Adopting some more of that would be beneficial I think regardless of the acrimony shown towards the user.js engine standard from a comment from another collaborator. GM has done very nicely at keeping up to date over the past few years and having clearly readable code with checks in place is probably a contribution to its continued success.

[Martii]

Closed by #267