From 7eed8e5b57a9aa504341c8f9b734f41c76fd3db6 Mon Sep 17 00:00:00 2001 From: Slaven Peles Date: Tue, 9 Apr 2024 14:37:10 -0400 Subject: [PATCH 1/5] Add brief style guidelines. --- CONTRIBUTING.md | 194 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 194 insertions(+) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 000000000..554bdc087 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,194 @@ +# Developer Guidelines + +## Code Style + +### Existing non-compliant code +The code that does not comply with these guidelines will be fixed in separate pull request(s). Any new contribution should follow closely these guidelines but should not change style in the existing GridKit code. + +### Error handling +Return values of member functions should be of type `int` and used for error handling. Functions return 0 if no error is encounter, return positive value for warnings and recoverable error, and negative value for irrecoverable errors. + +### Output +If an output is needed (for example, a warning needs to be displayed), use `std::cout` and not `printf` as shown below. There should be a space before and after each `<<`. + +```c++ +std::cout << "index out of bounds. Row " << i << " starts at: " << start << " and ends at " << end << std::endl; +``` + + +### Member variable naming + +Member variable names should use C-style name format and end with trailing underscore `_`. +```c++ +double member_variable_; // Yes +double another_member; // No, there is no trailing underscore to distinguish it from nonmember variables +double memberVariable_; // No, using lowercase camel instead of C-style name format +``` + +### Function names + +Use lowercase camel format for function names. +```c++ +int myFunction(double x); // Yes +int another_function(); // No, using C-style name format +int YetAnotherFunction(); // No, using uppercase camel name format +``` + +### Class names + +Class names should start with a capital letter. For instance, `Vector` and `Matrix` are valid class names, while `point` is not. + +### Enums (enumerated types) + +Always define `enum`s inside `GridKit` namespace. Type names should be capitalized and the constant names should be uppercase with underscores (but there is no underscore at the end!). + +```c++ + enum ExampleEnum { CONST_ONE = 0, + CONST_TWO = 8, + YET_ANOTHER_CONST = 17 }; +``` + +### Constants + +If a constant is used in more than one file, define it in `Common.h`. Constants names should be capitalized. + +```c++ + constexpr double Pi = 3.1415; // No, it should be all caps + constexpr double SQRT_TWO = 1.4142 // No, there is an underscore + constexpr double SQRTTWO_ = 1.4142 // No, there is an underscore + constexpr double EXP = 2.7183 // Yes +``` + +### Pointers and references + +The pointer `*` or reference `&` belong to the type and there should be no space between them and the type name. +```c++ +double* x; // Yes +int& n; // Yes +double *x, *y; // No, the pointer symbol is a part of `double*` type +int & n; // No, the reference symbol is a part of `int&` type +``` + +### Indentation +Use only spaces for indentation, not tabs. Indent size is 4 spaces. + +When defining a class, the code blocks after `private`, `public` and `protected` should be aligned with opening/closing braces. There should be an empty line before each definition (except the first one). See example below. +```c++ +class SomeClass +{ +public: + SomeClass(); + ~SomeClass(); + +private: + int some_variable_; + +protected: + void someFunction(); +}; +``` + +### Braces +All braces should follow Allman style: +```c++ +namespace someNamespace +{ + //some code +} +``` +For short functions (i.e., empty constructor), do not inline braces. +```c++ +ClassA::ClassA() +{ +} +``` +Have opening brace at the next line following `for`, `if`, or `while` statement. When using `else`, follow the example below. +```c++ +if (cond == true) +{ + // some code +} +else +{ + // some other code +} + ``` +Have a space between keywords `for`, `while` and `if` and the parenthesis as shown here: +```c++ +for (int i = 0; i < n; ++i) +{ + // some code +} +``` + +Do not use one-line `if`s and `for`s. Always use braces. + +### Use of spaces and newlines +There should be spaces between arithmetic operators. +```c++ +x = c * (a + b); //Yes +x = c*(a+b). // No, the clarity is better if there are spaces between binary operators and operands. +``` +When defining member functions, use one empty line between the functions. +```c++ +struct MyStruct +{ + int memberFunction() + { + // some code + } + + int anotherMemberFunction() + { + // some other code + } +}; +``` +Leave one empty line between all the includes and the first line of the actual code. +```c++ +#include + +int main() +{ + std::cout +} +``` + +Also, leave one empty line between `system` includes and `GridKit` includes, i.e., +```c++ +#include + +#include + +int main() +{ + //some code + return 0; +} +``` +The `system` includes should always be listed first. + +### Using namespaces +All classes should be in namespace `GridKit`. If needed, define additional namespaces inside `GridKit`. +```c++ +namespace GridKit +{ + class Solver // Yes, class defined inside GridKit namespace + { + // some code; + }; + + namespace LinearAlgebra + { + class Vector // Yes, class defined inside GridKit namespace + { + // vector code + }; + } +} + +class Matrix // No, class is outside GridKit namespace +{ + // matrix code +}; + From c63fbd882ecb78183dcd95c7fd77fbc875b53538 Mon Sep 17 00:00:00 2001 From: Slaven Peles Date: Mon, 11 Nov 2024 12:50:54 -0500 Subject: [PATCH 2/5] Respond to PR feedback. --- CONTRIBUTING.md | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 554bdc087..0fc253bf5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -15,8 +15,15 @@ If an output is needed (for example, a warning needs to be displayed), use `std: std::cout << "index out of bounds. Row " << i << " starts at: " << start << " and ends at " << end << std::endl; ``` +### Local variables naming -### Member variable naming +Local variable names should use C-style name format. +```c++ +double local_variable; // Yes +double localVariable; // No, using lowercase camel instead of C-style name format +``` + +### Member variables naming Member variable names should use C-style name format and end with trailing underscore `_`. ```c++ @@ -50,12 +57,14 @@ Always define `enum`s inside `GridKit` namespace. Type names should be capitaliz ### Constants -If a constant is used in more than one file, define it in `Common.h`. Constants names should be capitalized. +If a constant is used in more than one file, define it in `Common.h`. For +constants with long names, use underscores to separate words in the constant +name. Use all caps (screaming snake case). ```c++ - constexpr double Pi = 3.1415; // No, it should be all caps - constexpr double SQRT_TWO = 1.4142 // No, there is an underscore - constexpr double SQRTTWO_ = 1.4142 // No, there is an underscore + constexpr double Pi = 3.1415; // No, the constant name should be all caps + constexpr double SQRT_TWO = 1.4142 // Yes + constexpr double SQRTTWO_ = 1.4142 // No, there is a trailing underscore constexpr double EXP = 2.7183 // Yes ``` From eda2cc23966865beb11e923048898dcd588fe8d4 Mon Sep 17 00:00:00 2001 From: Slaven Peles Date: Tue, 10 Dec 2024 13:37:11 -0500 Subject: [PATCH 3/5] Add header files include guidelines. --- CONTRIBUTING.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0fc253bf5..2df13e50a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -15,6 +15,22 @@ If an output is needed (for example, a warning needs to be displayed), use `std: std::cout << "index out of bounds. Row " << i << " starts at: " << start << " and ends at " << end << std::endl; ``` +### Including header files + +Header files should be included in 3 separate blocks: standard libraries, +GridKit external dependencies, and GridKit header files. External libraries +should use `<...>`, while GridKit headers should be included with `"..."`. + +```c++ +#include // Standard lib headers +#include + +#include // GridKit dependencies +#include + +#include "Ida.hpp" // GridKit internal header +``` + ### Local variables naming Local variable names should use C-style name format. From 82cb13fbb110e98829d59b8bc96dfce49809b67c Mon Sep 17 00:00:00 2001 From: Slaven Peles Date: Fri, 13 Dec 2024 14:21:21 -0500 Subject: [PATCH 4/5] Update style guidelines. Add guidelines on line breaks. --- CONTRIBUTING.md | 106 +++++++++++++++++++++++++++++++++++------------- 1 file changed, 78 insertions(+), 28 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2df13e50a..69b271fe2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -3,32 +3,25 @@ ## Code Style ### Existing non-compliant code -The code that does not comply with these guidelines will be fixed in separate pull request(s). Any new contribution should follow closely these guidelines but should not change style in the existing GridKit code. +The code that does not comply with these guidelines will be fixed in separate + pull request(s). Any new contribution should follow closely these guidelines + but should not change style in the existing GridKit code. ### Error handling -Return values of member functions should be of type `int` and used for error handling. Functions return 0 if no error is encounter, return positive value for warnings and recoverable error, and negative value for irrecoverable errors. +Return values of member functions should be of type `int` and used for error +handling. Functions return 0 if no error is encounter, return positive value +for warnings and recoverable error, and negative value for irrecoverable +errors. ### Output -If an output is needed (for example, a warning needs to be displayed), use `std::cout` and not `printf` as shown below. There should be a space before and after each `<<`. +If an output is needed (for example, a warning needs to be displayed), use +`std::cout` and not `printf` as shown below. There should be a space before +and after each `<<`. If the line needs to be broken, the `<<` operators should +be aligned: ```c++ -std::cout << "index out of bounds. Row " << i << " starts at: " << start << " and ends at " << end << std::endl; -``` - -### Including header files - -Header files should be included in 3 separate blocks: standard libraries, -GridKit external dependencies, and GridKit header files. External libraries -should use `<...>`, while GridKit headers should be included with `"..."`. - -```c++ -#include // Standard lib headers -#include - -#include // GridKit dependencies -#include - -#include "Ida.hpp" // GridKit internal header +std::cout << "index out of bounds. Row " << i << " starts at: " << start + << " and ends at " << end << std::endl; ``` ### Local variables naming @@ -59,11 +52,14 @@ int YetAnotherFunction(); // No, using uppercase camel name format ### Class names -Class names should start with a capital letter. For instance, `Vector` and `Matrix` are valid class names, while `point` is not. +Class names should start with a capital letter. For instance, `Vector` and +`Matrix` are valid class names, while `point` is not. ### Enums (enumerated types) -Always define `enum`s inside `GridKit` namespace. Type names should be capitalized and the constant names should be uppercase with underscores (but there is no underscore at the end!). +Always define `enum`s inside `GridKit` namespace. Type names should be +capitalized and the constant names should be uppercase with underscores +(but there is no underscore at the end!). ```c++ enum ExampleEnum { CONST_ONE = 0, @@ -97,7 +93,9 @@ int & n; // No, the reference symbol is a part of `int&` type ### Indentation Use only spaces for indentation, not tabs. Indent size is 4 spaces. -When defining a class, the code blocks after `private`, `public` and `protected` should be aligned with opening/closing braces. There should be an empty line before each definition (except the first one). See example below. +When defining a class, the code blocks after `private`, `public` and `protected` +should be aligned with opening/closing braces. There should be an empty line +before each definition (except the first one). See example below. ```c++ class SomeClass { @@ -127,7 +125,8 @@ ClassA::ClassA() { } ``` -Have opening brace at the next line following `for`, `if`, or `while` statement. When using `else`, follow the example below. +Have opening brace at the next line following `for`, `if`, or `while` +statement. When using `else`, follow the example below. ```c++ if (cond == true) { @@ -138,7 +137,8 @@ else // some other code } ``` -Have a space between keywords `for`, `while` and `if` and the parenthesis as shown here: +Have a space between keywords `for`, `while` and `if` and the parenthesis as +shown here: ```c++ for (int i = 0; i < n; ++i) { @@ -151,7 +151,7 @@ Do not use one-line `if`s and `for`s. Always use braces. ### Use of spaces and newlines There should be spaces between arithmetic operators. ```c++ -x = c * (a + b); //Yes +x = c * (a + b); // Yes x = c*(a+b). // No, the clarity is better if there are spaces between binary operators and operands. ``` When defining member functions, use one empty line between the functions. @@ -179,7 +179,21 @@ int main() } ``` -Also, leave one empty line between `system` includes and `GridKit` includes, i.e., +Header files should be included in 3 separate blocks: standard libraries, +GridKit external dependencies, and GridKit header files. There should be an +empty line between the blocks. External libraries should use `<...>`, while +GridKit headers should be included with `"..."`. + +```c++ +#include // Standard libs headers +#include + +#include // GridKit dependencies +#include + +#include "Ida.hpp" // GridKit internal header +``` + ```c++ #include @@ -193,8 +207,44 @@ int main() ``` The `system` includes should always be listed first. +### Long function names + +Function declarations and function calls should be written on a single line. +If a function has large number of parameters, the function should be broken +over multiple lines so that each function parameter is on a separate line. +All parameters should be aligned with the first parameter in the function. + +```c++ +int myFunction(double x, double y); // Yes, function on a single line + +int myFunction(double x1, // Yes, function parameters on a separate lines. + double x2, + double x3, + double x4, + double x5, + double x6, + double x7); + +int myFunction(double x1, // No, function broken inconsistently. + double x2, double x3, + double x4, + double x5, double x6, double x7); + +int myFunction( + double x1, // No, first parameter should follow the parenthesis. + double x2, + double x3, + double x4, + double x5, + double x6, + double x7 +); + +``` + ### Using namespaces -All classes should be in namespace `GridKit`. If needed, define additional namespaces inside `GridKit`. +All classes should be in namespace `GridKit`. If needed, define additional +namespaces inside `GridKit`. ```c++ namespace GridKit { From 319c3514152e09a0ffc949d68ab308009faee4d6 Mon Sep 17 00:00:00 2001 From: Slaven Peles Date: Mon, 16 Dec 2024 10:59:09 -0500 Subject: [PATCH 5/5] Initializer list, file names, minor fixes. --- CONTRIBUTING.md | 65 +++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 60 insertions(+), 5 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 69b271fe2..6f6916d15 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -4,8 +4,8 @@ ### Existing non-compliant code The code that does not comply with these guidelines will be fixed in separate - pull request(s). Any new contribution should follow closely these guidelines - but should not change style in the existing GridKit code. +pull request(s). Any new contribution should follow closely these guidelines +but should not change style in the existing GridKit code. ### Error handling Return values of member functions should be of type `int` and used for error @@ -24,6 +24,13 @@ std::cout << "index out of bounds. Row " << i << " starts at: " << start << " and ends at " << end << std::endl; ``` +### File names + +Each class should be implemented in a `*.cpp` and a `*.hpp` files with +the same name. Files containing collection of standalone functions should +have a descriptive name starting with lowercase character. + + ### Local variables naming Local variable names should use C-style name format. @@ -52,8 +59,25 @@ int YetAnotherFunction(); // No, using uppercase camel name format ### Class names -Class names should start with a capital letter. For instance, `Vector` and -`Matrix` are valid class names, while `point` is not. +Class names should us uppercase camel name format. +```c++ +class MyClass // Yes +{ + ... +} + +class Matrix // Yes +{ + ... +} + +class My_Class // No, using underscore in class name +{ + ... +} +``` + + ### Enums (enumerated types) @@ -152,7 +176,8 @@ Do not use one-line `if`s and `for`s. Always use braces. There should be spaces between arithmetic operators. ```c++ x = c * (a + b); // Yes -x = c*(a+b). // No, the clarity is better if there are spaces between binary operators and operands. +x = c*(a+b). // No, the clarity is better if there are spaces between + // binary operators and operands. ``` When defining member functions, use one empty line between the functions. ```c++ @@ -169,6 +194,9 @@ struct MyStruct } }; ``` + +### Include files + Leave one empty line between all the includes and the first line of the actual code. ```c++ #include @@ -242,6 +270,33 @@ int myFunction( ``` +### Initializer list in class constructor + +Short initializer list should follow the constructor on the same line with +space around `:`. Long initializer lists should begin on the next line, +indented and starting with `:`. Initializers should be aligned. +```c++ +// Short initializer list +MyClass(n, m) : n_(n), m_(m) +{ + // ... +} + +// Long initializer list +MyClass(n, m) + : n_(n), + m_(m), + pX_(nullptr), + pY_(nullptr), + pZ_(nullptr), + size_(0) +{ + // ... +} +``` + + + ### Using namespaces All classes should be in namespace `GridKit`. If needed, define additional namespaces inside `GridKit`.