Coding Standards

[pjf]

We need 'em. Despite having a lot of experience in software development, I'm relatively new to C#, and would like standards/conventions that cause the least friction to other developers and potential developers.

My own background (Perl) has variables_with_underscores, but I've noticed C# examples predominantly use camelCase. Is there a commonly accepted standard on whether one should capitalise the first letter? In Perl the most commonly seen convention is that class variables and methods are capitalised, and instance methods and variables are not, although the enforcement of such varies from project to project.

It would also be lovely if there's a C# equivalent of Perl::Critic, which performs static analysis and can spot not only potential programming flaws, but also stylistic ones.

[NathanKell]

@eggrobin, the batsignal!

camelCase comes in part from KSP's own style for variable names; but I've found in C# and C++ alike it's good style (with methods and classes in CamelCase). I am far from stylistically versed in C#, however.

The closest thing I can think of might be ReSharper?

[eggrobin]

Summary of the conclusions from the IRC discussion:

While Microsoft provides naming suggestions, neither Unity nor KSP abide them, with KSP being strongly internally inconsistent (with inconsistency within a single identifier not being uncommon). As a result, any attempt at consistency with other modders or with the game is bound to fail instantly.
The Microsoft naming guidelines are far from a complete styleguide; no indications of formatting (tabs vs. spaces, line length, indent size, curly bracket placement, indent for multi-line expressions, newline placement on functions definitions and calls, etc.). While some styleguide exist that extend the Microsoft naming conventions, they do not enjoy the status of universal standard (most are not very thorough in their definition of the style).

Since we do not (or cannot) care about consistency with mutually and internally inconsistent external libraries, I suggested to @pjf the use of the Google C++ styleguide. While designed for another language, it is fairly easy to transpose, and it is quite detailed in its definition of a style.

An example of a transposition of a naming rule to C#:

Regular functions have mixed case; accessors and mutators match the name of the variable: MyExcitingFunction(), MyExcitingMethod(), my_exciting_member_variable(), set_my_exciting_member_variable().

In C#, the idiomatic way to implement accessors and mutators is properties (and conversely doing a lot of work in a property is bad form), so this directly translates to:

Functions have mixed case; properties are all lowercase, with underscores between words: MyExcitingFunction(), MyExcitingMethod(), my_exciting_property.

@pjf seems to like this style.

I have found that the existing tools for C# formatting generally lack in flexibility.
I would suggest doing systematic code reviews as we do in Principia, this would generally improve code quality and make it more likely to catch styleguide violations.

While we're considering code quality, we should address the question of unit tests. We should probably use NUnit (EDIT: apparently this has been thought of in #5), and start writing tests quickly, since a large codebase that was not written to be testable is essentially impossible to test after the fact (and an untested codebase is a buggy codebase).

[eggrobin]

Yet another question, which variant of English to use (for identifiers and comments). Principia uses en-GB-oed (colour, but initialize), but you may prefer en-US. Whichever one you chose, it would be good to mention a specific dictionary (en-GB-oed has the OED, perhaps we could use Merriam-Webster if we go with en-US?).
A decision must also be taken regarding 1 space vs. 2 spaces after full stops (for comments).

[eggrobin]

@pjf has decided to go with the Australian spelling. For convenience we shall define that as the main spelling given in the Macquarie Dictionary (colour, analyse, initialise).

[pjf]

I'd love to use StyleCop, because it does a great job of plugging into monodevelop and Visual Studio, but it seems to only have a single setting (utter pedantry), and I've no love for the default style it enforces. I like some things from the Google C++ style guide (local variables are lower-case with underscores, classes and methods CapitaliseEachWord), but not others (such as advising against exceptions, or the layout of functions with long arguments).

I'm on the fence about class attributes ending in an underscore, but that may be because I come from a language that requires an explicit this/self mention when referring to attributes.

In any case, the big things I agree on (and have been needing) are:

• ThisIsAClass.AndThisIsAMethod()
• this_is_a_local_variable

And (as @eggrobin mentioned), accessors and mutators should match the attributes they use.

This means I'll be changing a number of places where methodsLikeThis will become MethodsLikeThis.

---

As for more general style rules, I think we're going with this:

• We'll generally follow the Google C++ style guide.
• ...except with 4-space indents.
• ...and exceptions are cool, yo.
• ...and if you have a long parameter list, write it the way you feel comfortable with.
• ...and any other exceptions we make from time to time. :)

The over-riding rule is that clarity and maintainability trump anything in any of the style guidelines. Following a prescribed style to the detriment of maintainability is madness.

[eggrobin]

Sounds mostly good to me, comments below.

and exceptions are cool, yo.

This is sane in this small C# project. The main reasons why they are forbidden in the Google+ style are that:

1. Exceptions are broken in C++
2. Exceptions don't work in a large-scale codebase (huh, I got an exception 10 levels down the call stack), and are contagious (if someone decides to use an exception in bigtable, the entirety of the Google codebase has to either throw it upwards, and document that, or handle it somehow even if it is irrelevant).
   Instead, things just die if an invariant (glog CHECK macro) failed, or return a failure status.
   None of the above applies, so "throw considered harmful" makes no more sense than "goto considered harmful" in this project, it's just a matter of taste (for the record, I do not agree with an indiscriminate "goto considered harmful", Dijkstra's letter is no longer relevant).

4-space indents.

You'll run out of space quickly, but that's not otherwise a problem. You need to define how we indent a multiline expression though:

foo = bar + baz + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +
    + bbbbbbbbbbbbbbbbbb;  // 4-space indent.

would be what you would do with a 2-space general indent, should this become 4, 6, 8 spaces?

and if you have a long parameter list, write it the way you feel comfortable with.
Following a prescribed style to the detriment of maintainability is madness.

The above statement is the antithesis of a maintainable style :-)
The maintainer should always be consistent with the code they maintain.
"the way you feel comfortable with" does not imply being consistent with the surroundings, so it leads to inconsistent formatting within a file. What is a maintainer to do with that?
Amend the styleguide as you wish, but at least describe the acceptable alternatives. Preferably, choose one.

[pjf]

If an expression is so complex that it becomes unclear how to format it, then I question if it should be written in the first place.

In any case, I'm not particularly interested in arguing the finer points here. I've got a great resource (the Google C++ guidelines), which I can consult when I find myself scratching my head asking
"how do other projects do this?", and I'm very thankful for that. And as long as both contributors and myself are consistent with the code they write, I'm happy with them. If we start seeing that stylistic variations are posing a barrier to maintainability or new contributors joining, then of course we'll definitely want to address things then.

Thank you so much for your input, @eggrobin , I very much appreciate it.

~ Paul