Code Guidelines - lazychords/lazychords-lib GitHub Wiki

Here are some rules that the contributors of this project are required to follow in order to have their pull-requests accepted.

#Project Conventions ##Who does what and in what order

In order to have the least bugs possible, here is the order in which things should be done. If one of the steps has already been done, just go to the next step.

  1. Creation of a new feature
If you want to create a new feature (audio input for example), please make sure you have discussed it on the [mailing list](https://gitter.im/lazychords/lazychords-lib) and have presented what you wanted to do. If you have any non-trivial algorithms or need to use external librairies, it is important you mention it. Then, once your request has been approved by the code manager, you may move on.
  1. Public interface of the feature
You will need to suggest a public interface through headers for your feature. Add your name in the author of the headers of your feature. Please focus on making it easy to use for the user of the feature and make it easily extendable. Of course, when having questions about the design choices, do not hesitate to ask the code manager. Before moving on to the next step, make sure you have the approval of the code manager.
  1. Giving guidelines for the people who code
You will need to provide guidelines/brief documentation of your classes/functions so that people can implement them.
  1. Implementing the feature
Any member of the project can try to implement functions/classes of your feature. A person who contributes on a function must follow the following order.

2.    Start by adding to the documentation that you will implement that function and push it on git.
2.    Code the function
2.    Edit the documentation to make it precise. Push on git.
  1. Testing the feature
The person testing a function must not have coded the function and must test the function only by looking at the documentation (not the code). If any questions arise when writting the tests, send an email to the author of the function and ask for precisions in the documentation. Once the tests are written, add your name as the one who has tested this function. If some tests fails, check your tests (only using the documentation as a reference), and if they keep failing, do not do anything, the author of the function will notice and try to correct it. If he thinks there is an error in your tests, he will send you an email.

##Naming conventions ##Documentation conventions

Your documentation should be precise and efficient. To do so, we use Doxygen which helps formatting the documentation in a efficient way. However, Doxygen is just a tool and it is important that we find in your documentation enough information to :

  • Understand what the function does
  • Understand when the arguments passed are valid (template types are considered as arguments for this rule)
  • Understand what the function might modify (side effects)
  • Understand when the function might throw an exception
  • Know who wrote/tested this function

To help you do this, here is a quick summary of the common doxygen commands we will be using. You can also go here to find more commands.

  • Doxygen indications * @file filename Use this for each header ! * @class classname Avoid, doxygen can find this out on its own * @enum enumname Avoid, doxygen can find this out on its own * @typedef using A = B Avoid, doxygen can find this out on its own * @def MY_DEFINE Avoid, doxygen can find this out on its own * @fn R func(Arg1, Arg2) Avoid, doxygen can find this out on its own * @namespace namespacename Avoid, doxygen can find this out on its own * @internal Use this when the information should not be on the documentation
  • User documentation * @brief briefDesription Necessary * @details fullDescription When the brief description is not enough * @param Arg is ... Necessary for each parameter * @pre precondition description Necessary when assert checks needed * @post postcondition description Do not use this for return value * @invariant invariant description Necessary when assert checks needed * @return description Necessary for all functions returning non-void * @throws exception description Necessary for exceptions that might happen * @nothrow Use this only if the function can not throw any exception * @note information Gives information to the user such as time/space complexity * @warning information Informs of some dangerous/non-intuitive behavior * @example example Use this to illustrate what you mean * @property property Use this to indicate special interactions with other functions
  • Developper documentation * @author authorname Necessary * @todo task Do not hesitate to use this * @remark information Gives information to developpers about implementation * @tests authorname @ref linkToTestCase "tests" Necessary when tests have been written

Important remarks

  • Do not use @bug, use @todo instead
  • @note and @remark do not have the same meaning, @note is for the user, @remark is more about questions regarding the implementation. As one might try to search for @remark in the code, please chose wisely.
  • All files containing a doxygen documentation must have a description and use @file.
  • All public declarations must be documented. If something is not intended for public use, please put it in the namespace impl (except for macros...).
  • Check that your documentation appears in the html doxygen documentation
  • It is important you specify exceptions. Of course, as exceptions sometime happen within a function called by your program, it is very hard to keep track of all exceptions, but you should try to document the ones that might happen. There is no need to specify that you might throw an std::bad_alloc if you are just doing one push_back on a vector for example. However, if you are accessing a file through its name, you should specify the exception your program throws when the filename is invalid.
  • The order of the documentation should be Doxygen indications, User documentation, Develloper documentation.
  • Do not use @post for return values, use @return instead. Use @post if the functions modifies arguments, the class, or any other public information.
  • There is no need to precise class invariants as a postcondition.
  • If the function/class is template, the template parameter also appears in the class name/function prototype. Also, there must be a precondition explaining when the template type is valid.
  • Common functions, such as id, maxId, <<, >>, load, save, randomInstance, ... should be documented differently (I will explain this later).
  • Do not hesitate to use @todo !
  • Authors for files, classes, ... usually do not mean all that much as most of the work is done in the functions that have their own authors. They should represent the person in charge of the coherence of the file/class, usually the person responsible for the module.
  • Implementation files usually do not need to be commented as functions should be short and fairly straightforward. However, if this is not the case, consider explaining what you did with non doxygen C++ style comments.
  • Of course, you are welcome to use more commands from the doxygen documentation, but try to use those in the lists above if you can.

The special case of usual functions

In order to simplify the writting of documentation for common functions, the basic properties of the following functions have already been written.

  • void C::check() const In ConceptCheck
  • bool C::operator==(const C& other) const In ConceptEquality
  • bool C::operator!=(const C& other) const In ConceptEquality
  • static C C::randomInstance() In ConceptRandomInstance
  • static C C::load(std::istream&) In ConceptSerialize
  • void C::save(std::ostreal&) const In ConceptSerialize
  • static unsigned C::maxId() In ConceptId
  • static C C::fromId(unsigned) In ConceptId
  • unsigned C::id() const In ConceptId
  • operator<< In ConceptPrint
  • operator>> In ConceptRead

For all these functions, you should document them the following way.

@brief Usual functionName function following the @ref ConceptName "ConceptName Concept"
@details The special case of your function (grammar for << and >> for example, ...)
@pre if you have a precondition that is not already specified in the concept
@whatever if it adds something that is not already specified in the concept
@example If you have an example
@author
@test

##Testing conventions ##Debugging conventions #Language Restrictions ##No plain pointers

To ensure the safety of the code and a correct memory gestion (no memory leaks), we strongly discourage the use of plain pointers. For functions that needs to modify their arguments take them by (non const) reference. Do not forget to mention side-effects in the documentation! If you need an actual pointer mechanism, please use some pointer facility of the standard library (such as shared_ptr or unique_ptr, depending on what you intend to do)

##No bitwise operations except && and ||

Most bitwise operation (other than && and ||) are prone to errors and make code less readable (because some programmers are not use to C and its bitwise operations). Furthermore, most of the time, there is no performance differences as the compiler optimizes it. Of course, there can be exceptions (such as | for masks), but when you can avoid it, please do.

##No C-like casts

There is no point in using C-like casts as you can always replace it by a C++ cast that will make what you want to do clearer : C-like casts try to do C++ casts in the following order.

  1. static_cast
  2. reinterpret_cast
  3. const_cast

##Warnings

Try to avoid warnings as much as possible. Use the WARNALL option in Cmake to check for all warnings. If there is a warning that you believe should not be, ask the code manager before using #pragma to remove it.

##Do not use this->

Please use this-> to access to members only if you have to : it makes your code less readable.

##Floating numbers

Try to avoid floating numbers where you can and prefer the Fraction class. Also, do not use floating number functions on integers. For example, do not write the following code : static_cast<unsigned>(std::pow(static_cast<double>(x), 4.0))as it makes your code slower and more error prone.

##Global and Static variables

Global and static variables should not be used outside of a function, unless they are constexpr to avoid [fiasco in their initialisation order] (http://cpp.developpez.com/faq/cpp/?page=Les-donnees-et-fonctions-membres-statiques)

##Exceptions and ASSERT

The general rule is to use the ASSERT macro to detect programming errors, use exceptions to indicate that an unpredicted event occured. In practice, this means :

  • Use ASSERT to check preconditions
  • Use ASSERT to check invariants
  • Use exceptions to indicate that you can not do what you're asked to

Exceptions should not happen frequently, especially in the part of the code where performance is an issue. That is to say that you should never use an exception to replace a "if".

##Writting exception safe code

Learning to write exception safe code is hard. The idea behind it is that you should make sure that anywhere in the function, is you exit the function (an exception is thrown), everything is still in a valid state (no memory leaks, thread locking, invariants not respected, ...) or you are really sure that no exception can be thrown at that place. This is what is called Basic safety and any C++ function should provide that safety. More information on concepts & techniques to deal with exceptions can be found here

###Basic Safety

To achieve basic safety, the best way is to use RAII. For memory leaks, that means using smart pointers. For example, instead of doing.

Image::Image(const std::string& filename)
{
  data = getDataFromFile(filename); //Gets the pixels. data is a pointer to an array of pixels
  normalize(); //Something that normalizes the image for example. Assume that normalize can throw
}

And in the destructor of Image, you got delete data. Can you see the problem with the above code ? One solution is to make data a std::unique_ptr<pixel> instead of a pixel*. This correctly delete data, even when normalize throws.

Of course, the even better solution is just not to use new (or pointers in fact)... But sometimes, one might not be able to avoid it (same thing for locking variables in threads).

###Strong safety

Strong safety is the idea that either the function throws and everything is back to its original state or the function succeeds. This is much harder to ensure. In the following example:

void f(T& arg)
{
    arg = something;
    doSomethingThatCanThrow();
}

f might change the value of arg when an exception is thrown. So we need to reverse what has been done to arg.

However, if not specified by a @warning, we require all functions to provide strong safety (if doing so has huge impact on performance, just put in the @warning whatMightBeChanged).

Add a custom footer
⚠️ **GitHub.com Fallback** ⚠️