Variable name code style

[th-we]

I would like to change the following concerning variable naming:

• Eliminate snake case variable names and replace with camel case.
• Make all global variables (Self._property:* = ...) upper case, all local variables lower case.
• Remove the Self._property: prefix when reading global variables. Upper case will already make clear that this is a global variable.
• Eliminate re-assignments of global variables to local ones (like score = ActiveScore;), unless they are accessed a lot locally and it's clear that re-assignment has benefits (readability or performance).
• Document these conventions.

Anything else?

[ahankinson]

GlobalVariable vs localVariable?

Not a huge fan of the first, since I think of it more as a class or module name. Also a greater chance of collision with built-in Sibelius objects... Score = "blah blah".

Is there a reason to remove Self._property: prefixes?

Eliminate re-assignments of global variables to local ones ... and it's clear that re-assignment has benefits (readability or performance).

Tricky. Not sure how you would notice any reduction in readability or performance until it was too late. Generally moving global variables to the local scope improves performance -- at least it does in Python.

But it's one of those things that you probably wouldn't notice if you just did it one function at a time. So I'm not sure how you would measure the "unless" part of your rule.

I would say leave them alone. If you don't want to do it in new code, that's ok, but I wouldn't embark in a big refactor just to fix this, and I wouldn't make it a rule.

[ahankinson]

PS: Should we convert this to a discussion?

[ahankinson]

Or do you mean GLOBALVARIABLE?

That's also a bit tricky since camel case uses uppercase to separate words. So it would probably have to be snake format for globals: GLOBAL_VARIABLE.

I think some global variables start with an underscore now, don't they?

[th-we]

Or do you mean GLOBALVARIABLE?

That's also a bit tricky since camel case uses uppercase to separate words. So it would probably have to be snake format for globals: GLOBAL_VARIABLE.

Yes, I agree, but we don't have any of those right now. We only have this local variable:

sibmei/src/ExportGenerators.mss

Lines 201 to 202 in 2de9b4f

	FIRST_BAR = 1;
	mdiv = GenerateMDiv(FIRST_BAR);

But that variable is only used in those two lines and could be eliminated entirely.

I think some global variables start with an underscore now, don't they?

Good point. That's mostly the case for global TreeNode variables defined in GLOBALS.msd right now. We could either remove those underscores, or use the convention that global TreeNode variables defined in *.msd files should start with an underscore. Not sure if that's helpful.

Other than those TreeNode variables, we currently have _Initialized and _PluginList. _PluginList has the underscore because it is never used and just serves the purpose of keeping a reference to prevent garbage collection of the plugin objects, but we could remove the underscores from both those variables.

My main goal is to make it clear whether we're working with local or global variables right away. I don't care too much about the underscores. If the first letter after the underscore is upper case, I think it also makes clear we're dealing with a global variable.

[th-we]

GlobalVariable vs localVariable?

Not a huge fan of the first, since I think of it more as a class or module name.

As we can't access classes in ManuScript, that's not really an issue.

Also a greater chance of collision with built-in Sibelius objects... Score = "blah blah".

Score is not a predefined global variable, but I get your point.

Is there a reason to remove Self._property: prefixes?

Noise, and the plugin does not silently continue to run if the variable was not initialized, be it because the order of execution is not as intended (the function that does the initialization is only called after the first usage, or maybe never), or just because of a typo:

Self._property:FooBar = true;
// Typo
if (Self._property:FoooBar)
{
  Trace('We wanted this line to be printed, but it will not');
}

Self._property:FooBar = true;
// Typo
if (FoooBar)
{
  Trace('Now the plugin will show an error message instead of printing this line');
}

Tricky. Not sure how you would notice any reduction in readability or performance until it was too late.

Too late for what?

Generally moving global variables to the local scope improves performance -- at least it does in Python.

If we're talking performance, the break even of an additional assignment vs. direct usage of a global variable is when the variable is accessed about four times. Below, the global variable is likely faster, above, the local variable is faster. The Self._property: prefix however also has a slight performance impact, i.e. Self._property:Foo is more than 2x slower than just Foo.

[ahankinson]

But that variable is only used in those two lines and could be eliminated entirely.

It's probably there so that we don't have a magic number. I would leave it, if it's not hurting anything.

As we can't access classes in ManuScript, that's not really an issue.

No, but I noticed that function names are FunctionName. I'm not sure what happens if you name a global variable the same as a function -- it probably picks the one it sees last?

Underscore sounds ok, but I thought of a couple other suggestions. What about a common letter prefix? Self._property:GFooBar (like "IFooBar" for interfaces in TypeScript) or Self._property:g_FooBar? (That's like the m_ convention used in Verovio for marking class member variables). Sticking a "G" or a "g" (or any other character, it doesn't matter) would set it apart as a global variable, and would reduce the risk of name collision. You would, of course, double it for words that start with "G" -- "GGamba" or "GGlobalMeasures" -- just so the variable name stays unique.

Too late for what?

Too late in the refactoring process. I'm not sure what kind of impact not scoping external variables to an internal one has, but I expect that any performance impacts for individual calls will be small -- it's just adding them all up might be big.

I don't think it hurts anything to redeclare variables in a local scope. Like I said, you don't have to if you don't want to, but I'm not sure I would make it a rule.

Below, the global variable is likely faster, above, the local variable is faster.

That's really interesting! But I think this speaks even more to leaving it to your own judgement. Four times is likely to have minimal performance impact, but sometimes you just want to scope it locally and not care about how many times you call it. And if you do end up calling something ten times, then you've done the right thing without necessarily knowing it.

If you have that rule in your head, and want to do it, that's great! It could even be a suggestion or a "tip". But I wouldn't go so far as to "Eliminate" the practice. There are enough places in the code where it would be an exception that enforcing it as a "rule" would be hard.