Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consistent coding style #36

Closed
MarijnS95 opened this issue Sep 12, 2017 · 4 comments
Closed

Consistent coding style #36

MarijnS95 opened this issue Sep 12, 2017 · 4 comments
Labels
Area: dev Related to development environment Type: enhancement Issue is to be solved by implementing new features

Comments

@MarijnS95
Copy link
Member

MarijnS95 commented Sep 12, 2017

Put a styleguide together covering both formatting (eg. settings for the built in formatter) and syntactical matters.
Below is a list of inconsistencies and changes I've noticed in the project.

Formatting:

  • Spaces between keywords and parentheses if ( vs if(.
  • Unnecessary (trailing) white space (VS removes this on format).
  • Sorting of fields and methods (dependent on access modifiers mostly).
  • Use docstrings /// instead of regular (//) comments.
    • If needed, use the right syntax to refer to variables etc.

Syntactical / formatting (mixed):

  • Unnecessary use of this.
  • Braces around one liners (and ofc method body on a separate line). Single line method expressions are OK too.
  • Parens around assignment expressions foo = (bar != none);.
  • Unused (and unordered) using statements (VS can fix this as well).
  • Unnecessary casts.
  • Write field initializers at declaration instead of in the constructor (if applicable).
    public readonly List<string> SomeThings { get; } = new List<string>(); versus assigning it in the contstructor.
  • Short notations:
    • Null propagation such as foo = bar ?? cake;.
    • Ternary if statements such as foo = bar < 0 ? bar : foo;.
    • Safe navigation operator var foo = bla?.something;.

Safe code practices:

  • As strict as possible access modifiers (to indicate usage and prevent ambiguity):
    • Use static when a method doesn't (need to) touch local variables.
    • const and readonly where possible.
    • public vs internal, with multiple libraries going on this might be a good idea.
    • private on 'unity functions' that should not be called by our code (Awake, Start, etc)
    • Remove private set; when a property is only assigned in the constructor (or directly).
    • sealed classes? I'd say that's out of scope, until we start allowing plugins and such (or build a complete modding framework).
  • Check for NRE's everywhere;
  • Especially when retrieving datastructures from game-code. The game is constantly in development, and things will change. Defensive programming practices are a must to aid in debugging and correcting these changes later on.

Naming:

  • CTS or aliases (Int vs int and so on).
  • lowerCamelCase for everything private, UpperCamelCase for everything else?

Other:

  • Use of inline functions vs defining a full function with types etc.
  • Also applies to lambda notation: private bool cake() => foo != null;.
  • Sort Compile items in csproj files; this reduces faulty decisions in merge conflicts.
  • Use file-scoped namespace declarations for new code.

And remember, it's not so much a matter of preference as it is a matter of consistency.

@brianpopow
Copy link
Contributor

Most of those rules could be enforced with a stylecop ruleset: https://github.com/DotNetAnalyzers/StyleCopAnalyzers

Violations of those rules would then pop up as warnings (or errors if you like).

If you guys think that this would be useful, i could give it a try.

@ghost
Copy link

ghost commented Jan 12, 2021

I could go through the modules one by one and enforce this style.

@Measurity
Copy link
Collaborator

We should first discuss code style and create the code style files that visual studio and/or intellij use so they can enforce it. Fixing code style now will just lead to PRs changing it again which would be a waste of time.

@Measurity Measurity added Area: dev Related to development environment Type: enhancement Issue is to be solved by implementing new features labels Jun 12, 2022
@Measurity
Copy link
Collaborator

Closing as this got stale and fixing code-style is an ongoing process between the preferences of all Nitrox developers.

@Measurity Measurity closed this as not planned Won't fix, can't repro, duplicate, stale Dec 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: dev Related to development environment Type: enhancement Issue is to be solved by implementing new features
Projects
Status: Complete
Status: Complete
Development

No branches or pull requests

4 participants