Coding-Conventions-and-Guidelines.md

+NOTE: please carefully evaluate and give feedback. 
+
+**Page Content**<br>
+
+[[_TOC_]]
+
+# Project structure 
+
+The CORSIKA8 structure is set up following the guidance of other large software projects:
+- boost, e.g. https://github.com/boostorg/histogram  --> very similar project structure
+- NVIDIA/thrust: https://github.com/NVIDIA/thrust --> the use of ```*.inl``` files
+
+Advantages of this structure are:
+- clear correspondence and traceability of files/locations, in particular if they are ```#include```d
+- extremely clear separation between interface definition and documentation on one side, and implementation details on the other side
+- identical structure in ```source``` and ```build/install``` environment
+- very easy to deploy
+- very easy to manage with a well designed ```FindCCORSIK8.cmake``` tool. 
+
+Everything is relative to the (cmake) PROJECT_SOURCE_DIR source directory:
+
+ - ```corsika```   all header-only/ public headers - framework code
+ - ```modules```   all extra packages that needs to get compiles and then interfaced to the framework
+ - ```src```       the parts of the framework that needs building
+ - ```examples```  for all examples 
+ - ```test```      for all unit tests, sorted by components
+ - ```cmake```     cmake resources, build system
+ - ```externals``` needed crucial external packages
+ - ```documentation```   for Doxygen resources and configurations etc.
+ - ```tools```     extra tools and scripts
+
+More details: 
+
+the **/corsika** section is the header-only entry point for C8, and **/corsika/details** contains all corresponding inline files. 
+
+There is one main namespace: ```corsika```, all aspects of corsika8 are available there. Modules may create their own sub-namespaces if needed. 
+
+A process/module should contain code that even a typical bachelor student can comprehend and extend. A master student must be able to setup a new process from scratch, and do his research with this. 
+
+
+# Coding guidelines for CORSIKA 8
+
+- We use **clang-format** for basic code layout. Feel free to open discussions about its settings. 
+- We predominantly write code in **C++17**. We will move to C++20, when it is more established and supported. 
+- We write code motivated along well-known guidelines, please look at those:
+   - [High Integrity C++](https://www.perforce.com/resources/qac/high-integrity-cpp-coding-standard)
+   - [CppCoreGuidelines](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rc-org)
+   - [C++ standard library](https://style-guides.readthedocs.io/en/latest/cpp.html)
+
+Important rules, and notable differences are summarized here: 
+
+## For components coded in C++
+
+### Filenames, File Content and Layout
+
+- c++ header end on ```.hpp``` like in ```Interaction.hpp```. Headers files (```*.hpp```) should 
+  contain Doxygen documentation, macros, as well as class and function declarations. 
+- c++ files end on ```.cpp```  like in ```Interaction.cpp```. These files are the entry points 
+  for building executables and libraries.
+- c++ inline files end in ```.inl``` like in ```Interaction.inl```. Inline files ( ```*.inl```) should 
+  be post-  included, outside the concerning ```namespace``` in  the header files with same prefix-name 
+  and provide the  implementations for the classes and functions declared in the corresponding header. 
+  No Doxygen documentation should be stored in  ```*.inl``` files.  
+- All files that are meant for inclusion should be guarded with ```#pragma once``` in order to avoid 
+  multiple inclusion in the compilation units.
+- Header and Inline files are always included with ```<path/to/file.hpp>``` and not "path/to/file.hpp". 
+- Classes and files containing classes must have the same name. For example, the ```class Doer``` should be fully declared and documented in the header "Doer.hpp" and have their methods implemented in the file "Doer.inl".
+
+### Class, Namespace, Enum, Function, and Variable Naming Conventions
+
+- Free functions are small letters only like ```make_bar(Wine const& wine, Beer const& beer)```
+- Classes are named in upper camel case ```class LikeThis```
+- Class methods are lower camel case ```LikeThis::doSomething()```
+- Data members are lower camel case with a "_" suffix. ```LikeThis::parameterOne_```
+- Namespaces are small letters only like ```namespace this_namespace```
+- Enums follow class rules, e.g. ```enum class ThisIsAnEnum {one,two, three};```
+   - Define and use enums as locally as possible, as strongly namespaced as feasible (best inside their host class) 
+- Do not use abbreviations, if not absolutely necessary
+   - ```utility``` not ```utl```
+   - do not use e.g. ```i```, if  ```i``` is used in more than a single line of code; rather use ```index```, or ```count``` or whatever is appropriate 
+   - Same for ```p```, if it should be ```particle``` etc.
+
+### Class Design
+
+- Classes are designed like this:
+   - No public data member is allowed. In specific, limited, and obvious cases ```struct``` may be useful.
+   - Everything that does not need to be visible to the outside of a class/struct should be ```private``` or ```protected```. In particular member data MUST be non-public, but also consider each method, even constructors etc. 
+   - Everything that should not change should be ```const```
+   - Access data members using getters and setters
+      - for setters use either by value ```setValue(type value)``` or, if ```type``` is not an integral type by reference ```setValue(const type& value)```
+      - for getters use by value ```type getValue() const {return value_;}```, or by reference ```type& value() {return value_;}``` respectively ```const type& value() const {return value_;}```. Note, use the from ```value()``` ONLY when it returns a reference to a data member, i.e. for function chaining. 
+   - For logical states, use ```bool isTrue() const``` or ```bool hasThis() const```. Avoid ```bool isNotNeeded() const``` since it is just ```!isNeeded()```.  
+   - Data member declarations follow after method declarations.
+   - Classes need to be documented with doxygen commands. 
+   - Follow the ```rule of zero/five```. This concerns any of the following methods handling an object lifecyle:
+      - default constructor: ```X()```
+      - copy constructor: ```X(const X&)```
+      - copy assignment: ```operator=(const X&)```
+      - move constructor: ```X(X&&)```
+      - move assignment: ```operator=(X&&)```
+      - this is special, only if any memory allocation needs to be cleaned up, etc, a destructor ```~X()```
+
+     Either you implement any of them (zero), or all of them (five); and if memory/resources must be released also the destructor.  
+      Instead of implementing, you may use the defaults ``` = default``` or ``` = delete``` keywords to indicate desired behavior. 
+
+  These simple rules are summarized in the snippet below:
+  ```
+  // forward declarations
+  class Beers;
+  class Wine;
+
+  /* \class Bar
+   * \brief Just a very limited bar...  
+   */
+  class Bar 
+  {
+   ///! trivial constructor
+   Bar() = default; 
+ 
+   ///! non-trivial constructor
+   Bar( Beer const& beer, Wine const& wine):
+     beer_(beer),
+     wine_(wine)
+     {}
+ 
+  ///! move constructor
+  Bar( Bar && other) = delete;
+
+  ///! copy constructor
+  Bar( Bar const& other):
+    beer_(other.getBeer()),
+    wine_(other.getWine())
+    {} 
+
+  ///! assignment operator
+  Bar& operator=( Bar const& other){
+    if(this=&other) return *this;
+    beer_ = other.getBeer();
+    wine_ = other.getWine();
+    return *this;
+   } 
+
+   void DrinkAll(void);
+
+   Beer getBeer() const {
+      return beer_;
+   }  
+
+   void setBeer(Beer const& other){
+      beer_=other;
+   }  
+
+   // for function chaining:
+   Bar& beer(Beer const& other){
+      beer_=other;
+      return *this;
+   }  
+  
+   Wine getWine(){
+      return wine_;
+   }  
+
+   // for function chaining:
+   Wine& wine(){
+      return wine_;
+   }  
+
+   void setWine(Wine const& other){
+      wine_=other;
+    }
+  
+  // data members last
+  private:
+    Beer beer_;
+    Wine wine_;
+  };
+  ```
+  Reflect about the resources the class is holding and if makes sense to move or copy them. 
+  Implement non-default or long components in the *.inl  
+
+- Each functional group of classes has a unit test ```testThis.cpp```
+- Adhere to C++17 standards. In particular:
+   - Define public, private and protected type aliases and typedefs on the top, inside the class or 
+     method they are
+     necessary. It should be written in minor letters, suffixed by "_type", like in ``` typedef 
+     ClassA someclass_type```.
+   - Template, meta programming: 
+      - Template (meta) parameters are identified by a prefixed "T", like ```template <typename TProperty>```. 
+        This makes it easier in writing code to distinguish classes and types from template (meta) parameters.
+   - For class templates, provide deduction guides, if applicable. Also, add instance makers 
+     as free functions in the concerning namespace, to deploy type deduction and avoid explicit 
+     template instantiation. Look the example below:
+     ```
+     // prototype example of a class to manage process queue, that should be able to 
+     // dispatch processes in parallel o sequentially, according to TParallelPolicy 
+     // this simple design allow to compose tasks in groups with different policies
+     template<typename ParallelPolicy, typename ProcessList, typename StackIterator>
+     class ProcessQueue;
+
+     // specialization and concrete implementation
+     template<template< typename ...T > ParallelPolicy, typename StackIterator, typename ...Processes>
+     class ProcessQueue<ParallelPolicy<T...>, std::tuple<Processes...>, StackIterator >: 
+      public ProcessDispacher<<ParallelPolicy<T...>>
+     {
+     public:
+      typedef ParallelPolicy<T...>                    dispatch_policy;
+      typedef ProcessDispacher<<ParallelPolicy<T...>> dispatcher_type;
+      typedef std::tuple<Processes...>             process_tuple_type;
+      typedef std::pair<StackIterator>               stack_range_type;
+     
+     // constructor (0)
+     ProcessQueue() = delete;
+
+     // constructor (1)
+     ProcessQueue(dispatch_policy const& policy, process_tuple_type const& processes, stack_range_type const& range ):
+         TProcessDispacher<<TParallelPolicy<T...>>(policy),
+         processes_(processes),
+         stack_range_()
+         {}
+
+     // constructor (2)
+     template<typename Stack, typename = typename std::enable_if<detail::is_stack<Stack>>::type >
+     ProcessQueue(dispatch_policy const& policy, process_tuple_type const& processes, Stack const& stack ):
+         TProcessDispacher<<TParallelPolicy<T...>>(policy),
+         processes_(processes),
+         stack_range_(std::make_pair(std::forward<TStack>().begin(),
+           std::forward<TStack>().end()))
+         {}
+      /* 
+      .
+      .  other methods, ctors, etc
+      .
+      */
+      
+      void operator()(){
+        return dispatcher_type::callEach(stack_range_, processes_ );
+      }
+      
+      private:
+       processes_tuple_type processes_;
+       stack_range_type     stack_range_;
+      };
+
+      // deduction guide
+      template<typename TParallelPolicy, typename TStack, typename ...TProcesses>    
+      ProcessQueue( dispatch_policy const& policy, process_tuple_type const& processes, Stack const& stack)->
+      ProcessQueue<TParallelPolicy, std::tuple<TProcesses...>, declatype(std::declval<TStack>().begin()) >;
+
+      // instance maker
+      template<typename TParallelPolicy, typename TStack, typename ...TProcesses>
+      auto make_process_queue(TParallelPolicy const& policy, TStack&& stack, TProcesses const&...processes )
+          -> ProcessQueue<TParallelPolicy, std::tuple<TProcesses...>, declatype(std::declval<TStack>().begin()) >
+      {
+      return ProcessQueue( policy, std::make_tuple(processes...),
+       std::make_pair(std::forward<TStack>().begin(), std::forward<TStack>().end()) );
+      }  
+
+     ```
+
+     Technical references:   
+
+      1. https://en.cppreference.com/w/cpp/language/template_argument_deduction
+      2. https://en.cppreference.com/w/cpp/language/class_template_argument_deduction
+
+
+## General Rules
+
+- Warnings have to be fixed, when they appear on the system(s) currently used by the CI, by altering the code unless the warning originates from third-party code and cannot be fixed. In that case, the warning should be locally silenced with appropriate pragmas or by locally turning off warnings for that translation unit. 
+- All unit tests must succeed on the specified systems/configurations on gitlab-ci. If tests fail, code is not merged.  Consider to update, extend the CI systems! 
+   - The unit test code coverage should never decrease due to a new merge request. 
+- Exceptions and error handling
+   - On major, but rare, errors or malfunction we throw an exception. This is needed and required for complex physics and shower debugging. 
+   - We don't catch exceptions for error handling, there might be very few special exceptions from this. We need to discuss such cases. We want to use exceptions as well defined entry points into e.g. gdb
+- References, Pointers
+   - We prefer the use of references, wherever useful
+   - There cannot be any raw pointer in the interface of any class or object
+     exposed to outside users, there might be (technical) raw pointers for very special (exceptional) cases
+     inside of classes.
+   - If needed, revert to std::unique_ptr, or std::shared_ptr (but sparsely).
+- When you contribute new code, or extend existing code, at the same time provide unit-tests for all functionality.
+- When you contribute new physics algorithms, in addition you also need to provide a validation module (this is still TBD what exactly this means)
+
+### Comments and Documentation
+
+- Declared code must be documented with ```doxygen``` commands extensively within the public header files.  
+- There should not be any useless comments in code, in particular absolutely avoid to commit commented-out debug remnants
+  ```
+  thisIs = used_code;
+  // thisWas(used_for_debug_only);   //  <---    This line has to be removed
+  ```
+- Add sufficient and meaningful comments to the code (only in English), but only if the code is not self-explanatory. Better write your code, so that it is clear what is done, for example:
+  ``` 
+  TimeType TimeOfIntersection(Line const& line, Plane const& plane) {
+      auto const line_direction = line.GetDirection();     
+      auto const plane_normal = plane.GetNormal();     
+      return plane_normal.dot(plane.GetCenter()-line.GetPosition()) / plane_normal.dot(line_direction);     
+      }
+  ```
+  and not 
+  ```
+  TimeType TimeOfIntersection(Line const& l, Plane const& p) {
+      auto const d = p.GetCenter() - l.GetR0();     
+      auto const v = l.GetV0();     
+      auto const n = p.GetNormal();     
+      auto const c = n.dot(v);    
+      return n.dot(d) / c;     
+      }
+  ```
+
+
+## For components not coded in C++
+
+ - fortran files end on ".f" ```sibyll23c.f```
+ - python files end with ".py"
+ - C header files ends with ".h" and source files with ".c"
+
+
+## CMAKE formatting
+
+  - command are lower cases, e.g. ```set (...)```
+  - variables upper case, e.g. ```set (VAR1 Text)```
+
+Since cmake itself lacks structure almost entirely:
+  - put a space between command and start of parenthesis, e.g. ```command (...)``` 
+  - add two spaces for logical indent 
+    ```
+    if (condition)
+      do something
+    endif (condition)
+    ```
+  - break long lines to start with new keyword in new line (indented)
+    ```
+    install (
+      FILES ${CORSIKAstackinterface_HEADERS}
+      DESTINATION include/${CORSIKAstackinterface_NAMESPACE}
+      )  
+    ```
+  - add plenty of comments to all cmake code
+  - use expressive variables and functions
+