From 3ef394686d108d977d74ca9ec7856c6d739cb16b Mon Sep 17 00:00:00 2001 From: Matt Nadareski Date: Fri, 15 May 2026 13:05:42 -0400 Subject: [PATCH] Update CONTRIBUTING to be more accurate --- CONTRIBUTING.md | 219 +++++++++++++++++++++++++++++++----------------- 1 file changed, 144 insertions(+), 75 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f07f6af..9e83071 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,26 +1,24 @@ # Coding Guide -This document serves as the official code standards guide development. Please note that this is a work in progress and may not encapsulate all standards expected of new or existing code. The included `.editorconfig` file can help enforce some of the standards mentioned below. +This document serves as the official code standards guide development. Please note that this is a work in progress and may not encapsulate all standards expected of new or existing code. The included `.editorconfig` file can help enforce many of the standards mentioned below. -## General Code Guidelines +There are no requirements on a coding environment or IDE in order to contribute. If there are issues with a particular environment, please surface an issue with the project. If there are support files or updates needed to items such as the `.gitignore`, please open an issue or PR to assist the maintainers. Consider using an editor that can take advantage of the `.editorconfig` file to enable automatic enforcement. -This section contains information on code standards regardless of which part of the project you are working in. +## External Code -### Style and Naming +Generally, the `System` and `SabreTools` libraries are going to be preferred over all external code. If external code is needed, reasonable alternatives need to be explored before it will be included. External code use can also be seen as an opportunity to include that functionality in a `SabreTools` project directly. -- Prefer `System` namespaces for supporting operations before external ones. +External code should first be considered in the form of a Nuget package to allow for easier versioning and tracking. If this is not viable, consult with a maintainer to see if including the code as a Git submodule or having a direct copy of the code included would be reasonable. -- Ordering of `using` statements goes: - - `using System.*` - - `using ` - - `using static ` - - `using X = Y` +## General Coding Style + +There are some general coding style preferences that should be adhered to: - Use 4 spaces for `tab`. - Curly braces should generally start on the line after but inline with the start of the previous statement, even if multiline. - ```c# + ```csharp if (flag) { DoSomething(); @@ -34,7 +32,7 @@ This section contains information on code standards regardless of which part of - Multi-line statements need to have following lines indented by one step at minimum. - ```c# + ```csharp if (flag) { DoSomething(); @@ -48,31 +46,17 @@ This section contains information on code standards regardless of which part of } ``` -- Methods and classes should use `PascalCase` for naming, even `internal` and `private` ones. - -- Class properties should use `PascalCase` for naming, even `protected` and `internal` ones. - -- Instance variables should use `camelCase` with a `_` prefix for naming, even `protected`, `internal`, and `private` ones. - -- In-method variables should use `camelCase` without a `_` prefix for naming. - -- Include explicit access modifiers for all class-level properties, variables, and methods. - -- Avoid making everything `public`; only include the necessary level of access. - -- Avoid making every method and class instance-based. Use `static` if your method does not need to access instance variables. Use `static` if your class only contains extensions or methods used by other classes. - - Null-coalescing and null-checking operators can be used to make more readable statements and better get across what a statement or string of statements is doing. - ```c# + ```csharp if (obj?.Parameter is not null) { ... } bool value = DoSomething() ?? false; ``` -- `#region` tags, including nested ones, can be used to both segment methods within a class and statements within a method. Indentation follows the surrounding code. +- `#region` tags, including nested ones, can be used to both segment methods within a class and statements within a method. Indentation follows the surrounding code. These should be preferred over comments that indicate large blocks of related operations. - ```c# + ```csharp #region This is the first region public static void Method() @@ -89,17 +73,95 @@ This section contains information on code standards regardless of which part of #endregion ``` -- Try to avoid use of other preprocessor directives unless consulting ahead of time with the maintainers. +- Try to avoid use of preprocessor directives other than `#region`, `#endregion`, `#if`, `#elif`, and `#else` unless consulting ahead of time with the maintainers. Exceptions may be made for copied `#pragma` use, especially for known style violations. -- Interfaces should be listed in alphabetical order +## Naming Conventions - ```c# +With the exception of directly-included external code, the following naming conventions should be observed: + +- Methods and classes should use `PascalCase` for naming, even `internal` and `private` ones. + +- Properties should use `PascalCase` for naming, even `protected` and `internal` ones. + +- Fields should use `camelCase` with a `_` prefix for naming, even `protected`, `internal`, and `private` ones. + +- Constants should use `PascalCase` for naming, even `protected`, `internal`, and `private` ones. + +- In-method variables should use `camelCase` without a `_` prefix for naming. + +- Avoid using `_` in the middle of any names that are included in library code. The exception to this rule are test methods where they are used to separate parts of the test description. e.g. `public void MethodName_NullInput_False() { ... }` + +## Access Modifiers + +Generally, all classes, methods, properties, and fields should include explicit access modifiers. This applies even if the assigned access modifier would be the same as the default value in C\#. Below are some other guidelines to be aware of around access: + +- Avoid making everything `public`; only include the necessary level of access, including accounting for any tests that need to be written. + +- Avoid making every method and class instance-based. Use `static` if your method does not need to access instance variables. Use `static` if your class only contains extensions or methods used by other classes. + +## `using` Statements + +Ordering of `using` statements goes: + +```csharp +// System.* namespaces in alphabetical order +using System; +using System.Collections; +using System.Collections.Generic; +using System.Numerics; + +// External namespaces in alphabetical order +using Newtonsoft.Json; +using SabreTools.IO; +using SabreTools.IO.Extensions; +using Xunit; + +// Static usings in alphabetical order +using static SabreTools.Data.Models.BFPK.Constants; +using static SabreTools.Data.Models.PKZIP.Constants; + +// Renamed usings in alphabetical order of local name +using CompressionMode = System.IO.Compression.CompressionMode; +using IOSeekOrigin = System.IO.SeekOrigin; +``` + +There should be no comments or additional newlines in between `using` statements. The newlines and comments in the above example are to help explain, rather than act as a direct guide. + +There may be a need to version-gate some `using` statements if only certain .NET versions require particular code. If this is the case, all namespaces are still listed in the order above, with the preprocessor directives directly contained within. For example: + +```csharp +using System; +#if NETCOREAPP +using System.Collections.Concurrent; +#else +using System.Collections.Generic; +#endif +using System.IO; +``` + +## Classes + +Classes are considered to be logical groupings of functionality and should be treated as public-by-default. All non-private classes should be put into their own separate files. This includes typed-overrides, e.g. `Thing` and `Thing`. Below are some other general guidelines for classes: + +- Static classes are allowed, but they should not be the default case. + +- Partial classes are allowed and are even recommended when there needs to be further logical breakdown of functionality. Common uses of partial classes include separating out interface implementations. + +- If multiple interfaces are implemented, they should be listed in alphabetical order. If there are exceptions to this, they will be mentioned in project-specific developer guides. + + ```csharp public class Example : IBindable, IComparable, IEquatable ``` +- Private embedded classes are allowed, but should generally avoided if at all possible. This helps to both avoid discoverability issues as well as better help enforce separation of concern. If an embedded class is desired, talk to a maintainer before doing so. + +## Methods + +Methods should attempt to limited in size and scope wtihin reason. Methods that do too much are at risk of being untestable or cause maintenence issues. If a method has logical groupings of statements that could act as a standalone method by themselves, then consider doing so. Below are some other general guidelines for methods: + - Use the `` tag when possible to avoid out-of-date information. - ```c# + ```csharp public interface IInterface { /// @@ -115,41 +177,50 @@ This section contains information on code standards regardless of which part of } ``` -### Methods - - Try to avoid including too much duplicate code across methods and classes. If you have duplicate code that spans more than ~5 lines, consider writing a helper method. - Try to use expressive naming. e.g. use names like `PrintSectionTitles` and not `DoTheThing`. -- Try to avoid having too many parameters in a method signature. More parameters means more things interacting. +- Try to avoid having too many parameters in a method signature. More parameters means more possible compounded issues. -- Use method overloading to avoid unnecessary complexity in a single method. +- Use method overloading if there are multiple, fully-distinct paths within the method. This can help avoid unnecessary complexity in a single method. - ```c# - Instead of: + ```csharp + // Instead of: + public void Print(string? idString, byte[]? idArray, int? idInt) + { + if (idString is not null) { ... } + else if (idArray is not null) { ... } + else if (idInt is not null) { ... } + } - Print(string idString, byte[] idArray, int idInt) { ... } + // You should: + public void Print(string? id) { ... } - You should: + public void Print(byte[] id) { ... } - Print(string id) { ... } - - Print(byte[] id) { ... } - - Print(int id) { ... } + public void Print(int id) { ... } ``` -- Use optional parameters when the default value is the most common. +- Use optional parameters when the default value is the most common. Method overloads are also acceptable to use here as well, depending on developer and maintainer preference. - ```c# - Print(string id, bool toLower = false) { ... } + ```csharp + // Using default parameters + public void Print(string id, bool toLower = false) { ... } + + // Using method overloads + public void Print(string id) => Print(id, toLower: false); + + public void Print(string id, bool toLower) { ... } ``` -### `if-else` and `switch` statement syntax +## `if-else` and `switch` statement syntax + +There are multiple approaches to selections within code. There is often not a "best" solution, but the following guidelines should help guide toward a solution that both developers and maintiners can agree on. - If all statements in the block are single-line, do not include curly braces. - ```c# + ```csharp if (flag) DoSomething(); else if (flag2) @@ -160,7 +231,7 @@ This section contains information on code standards regardless of which part of - If any of the statements is multi-line _or_ the `if-else` statement is multi-line, include curly braces. - ```c# + ```csharp if (flag) { DoSomething(); @@ -180,9 +251,8 @@ This section contains information on code standards regardless of which part of - If comparing against values, try to use a `switch` statement instead. - ```c# - As an if-else statement: - + ```csharp + // As an if-else statement: if (value == 1) DoValue1(); else if (value == 2) @@ -192,8 +262,7 @@ This section contains information on code standards regardless of which part of else DoValueDefault(); - As a switch statement: - + // As a switch statement: switch (value) { case 1: @@ -211,11 +280,10 @@ This section contains information on code standards regardless of which part of } ``` -- If comparing against values for assignment, try to use a `switch` expression instead. +- If comparing against values for assignment, try to use a `switch` expression instead. Please note that this may not always be possible depending on the types that are being compared. - ```c# - As an if-else statement: - + ```csharp + // As an if-else statement: int x; if (value == 1) x = 0; @@ -226,8 +294,7 @@ This section contains information on code standards regardless of which part of else x = -1; - As a switch statement: - + // As a switch statement: int x = value switch { 1 => 0, @@ -237,9 +304,9 @@ This section contains information on code standards regardless of which part of } ``` -- When using a `switch` statement, if all switch cases are single-expression, they can be written in-line. You can also add newlines between cases for segmentation or clarity.If the expressions are too complex, they should not be. +- When using a `switch` statement, if all switch cases are single-expression, they can be written in-line. You can also add newlines between cases for segmentation or clarity. If a single case ends up overly-complex, consider separating it out into a new method and then calling that method instead. - ```c# + ```csharp switch (value) { case 1: DoValue1(); break; @@ -252,7 +319,7 @@ This section contains information on code standards regardless of which part of - When using a `switch` expression, cases that lead to the same value can be combined using `or`. This is not required, especially if readability would be sacrificed. - ```c# + ```csharp int x = value switch { 1 or 2 => 0, @@ -264,7 +331,7 @@ This section contains information on code standards regardless of which part of - If any of the switch cases are multi-expression, write all on separate lines. You can also add newlines between cases for segmentation or clarity. - ```c# + ```csharp switch (value) { case 1: @@ -284,11 +351,13 @@ This section contains information on code standards regardless of which part of } ``` -### Commenting +## Commenting + +Comments can be essential to the understanding of a given block of code. They can also be very helpful for figuring out appropriate use of the code and even sometimes help track down issues. Below are some other general guidelines for commenting: - All classes and methods should contain a `summary` block at bare minimum to explain the purpose. For methods, it is highly recommended to also include `param` tags for each parameter and a `return` tag if the method returns a value. Do not hesitate to use `remarks` as well to include additional information. - ```c# + ```csharp /// /// This class is an example /// @@ -317,7 +386,7 @@ This section contains information on code standards regardless of which part of - In-code comments should use the `//` syntax and not the `/* */` syntax, even for multiple lines. - ```c# + ```csharp // This code block does something important var x = SetXFromInputs(y, z); @@ -327,20 +396,20 @@ This section contains information on code standards regardless of which part of var w = SetWFromInputs(x, y, z); ``` -- Comments should be expressive and fully explain what is being described. Try to avoid using slang, "pointed comments" such as "you should" or "we do". +- Comments should be expressive and fully explain what is being described. Try to avoid using slang or statements that may have double meanings. If you ware unsure if a comment will be read incorrectly, please consult with a maintainer. -- Comments should avoid the use of first-person writing, such as "I think" or "We found". +- Comments should avoid the use of first-person writing and "pointed comments", such as "I think", "We found", or "You should". -- If comments include links, they can either be included as-is or using the `` tag +- If comments include links, they can either be included as-is or using the `` tag. If the link is being included in a class- or method-level comment, prefer using the latter option. - ```c# + ```csharp // This information can be found from the following site: // ``` - Try to avoid using multiple, distinct comment blocks next to each other. - ```c# + ```csharp // We want to try to avoid this situation where // we have multiple things to say.