OpenUserJS / OpenUserJS.org

The home of FOSS user scripts.
https://openuserjs.org/
GNU General Public License v3.0
858 stars 305 forks source link

Prefix all declared function parameter list identifiers with `a` #264

Closed Martii closed 10 years ago

Martii commented 10 years ago

Applies to #262

Zren commented 10 years ago

Wait, we're now using hungarian notation?

Martii commented 10 years ago

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 commented 10 years ago

We're only prefixing function parameter names. Nothing else. On Jul 14, 2014 4:09 PM, "Chris Holland" notifications@github.com wrote:

Wait, we're now using hungarian notation?

— Reply to this email directly or view it on GitHub https://github.com/OpenUserJs/OpenUserJS.org/issues/264#issuecomment-48960052 .

Martii commented 10 years ago

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

Correct.

Zren commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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

Martii commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

many projects already have in place.

Name 1

Martii commented 10 years ago

... 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 commented 10 years ago

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 UPPERCASE. 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:

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 commented 10 years ago

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.

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

aErr, aCb, aRes, aRep, aNext

Short forms are now capitilzed and they look terrible.

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 commented 10 years ago

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:


@sizzlemctwizzle Inconsistencies found:

@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 commented 10 years ago

Closed by #267