search

wocommunity / wolips

wolips
53 stars 53 forks source link

Improve Wonder templates #130

Closed darkv closed 7 years ago

darkv commented 7 years ago

This patch adds Type information to generated ERXKeys. Additionally static variables should be in uppercase.

spelletier commented 7 years ago

All uppercase is usually reserved for constant. It is very odd to use an all cap variable name as an invocation target. Even constant are now often named in CamelCase with an uppercase first letter in modern code. I suggest to keep log as is but keep the other commit.

darkv commented 7 years ago

Isn't something declared as static final a constant? Most code checkers like SonarQube will mark the lowercase log variable as bug / code smell / bad practice. But I can create a new pull request without that change.

spelletier commented 7 years ago

It is true that static final is a constant for the compiler. But if it contains a reference to a mutable object, it does not represent a constant by definition. I prefer to use human definition as these rules are to help human not compilers (they do not care about coding style...)

Official Google coding style is to use all caps for real constants including reference to immutable container but camel case for static final to mutable objects. I often see this convention is modern code. Personally, I hate all cap (hard to read) so much that I often name my class private constants like classes. Any IDE will colour them correctly anyway.

For this particular case, the log variable is always log in Wonder... Any other opinions out there ?