solvespace/CONTRIBUTING.md

Contributing to SolveSpace
==========================

Contributing bug reports
------------------------

Bug reports are always welcome! When reporting a bug, please include the following:

  * The version of SolveSpace (use Help → About...);
  * The operating system;
  * The save file that reproduces the incorrect behavior, or, if trivial or impossible,
    instructions for reproducing it.

GitHub does not allow attaching `*.slvs` files, but it does allow attaching `*.zip` files,
so any savefiles should first be archived.

Contributing code
-----------------

SolveSpace is written in C++, and currently targets all compilers compliant with C++11.
This includes GCC 5 and later, Clang 3.3 and later, and Visual Studio 12 (2013) and later.

### High-level conventions

#### Portability

SolveSpace aims to consist of two general parts: a fully portable core, and platform-specific
UI and support code. Anything outside of `src/platform/` should only use standard C++11,
and rely on `src/platform/unixutil.cpp` and `src/platform/w32util.cpp` to interact with
the OS where this cannot be done through the C++11 standard library.

#### Libraries

SolveSpace primarily relies on the C++11 STL. STL has well-known drawbacks, but is also
widely supported, used, and understood. SolveSpace also includes a fair amount of use of
bespoke containers List and IdList; these provide STL iterators, and can be used when
convenient, such as when reusing other code.

One notable departure here is the STL I/O threads. SolveSpace does not use STL I/O threads
for two reasons: (i) the interface is borderline unusable, and (ii) on Windows it is not
possible to open files with Unicode paths through STL.

When using external libraries (other than to access platform features), the libraries
should satisfy the following conditions:

  * Portable, and preferably not interacting with the platform at all;
  * Can be included as a CMake subproject, to facilitate Windows, Android, etc. builds;
  * Use a license less restrictive than GPL (BSD/MIT, Apache2, MPL, etc.)

#### String encoding

Internally, SolveSpace exclusively stores and uses UTF-8 for all purposes; any `std::string`
may be assumed to be encoded in UTF-8. On Windows, UTF-8 strings are converted to and from
wide strings at the boundary; see [UTF-8 Everywhere][utf8] for details.

[utf8]: http://utf8everywhere.org/

#### String formatting

For string formatting, a wrapper around `sprintf`, `ssprintf`, is used. A notable
pitfall when using it is trying to pass an `std::string` argument without first converting
it to a C string with `.c_str()`.

#### Filesystem access

For filesystem access, the C standard library is used. The `ssfopen` and `ssremove`
wrappers are provided that accept UTF-8 encoded paths.

#### Assertions

To ensure that internal invariants hold, the `ssassert` function is used, e.g.
`ssassert(!isFoo, "Unexpected foo condition");`. Unlike the standard `assert` function,
the `ssassert` function is always enabled, even in release builds. It is more valuable
to discover a bug through a crash than to silently generate incorrect results, and crashes
do not result in losing more than a few minutes of work thanks to the autosave feature.

### Use of C++ features

The conventions described in this section should be used for all new code, but there is a lot
of existing code in SolveSpace that does not use them. This is fine; don't touch it if it works,
but if you need to modify it anyway, might as well modernize it.

#### Exceptions

Exceptions are not used primarily because SolveSpace's testsuite uses measurement
of branch coverage, important for the critical parts such as the geometric kernel.
Every function call with exceptions enabled introduces a branch, making branch coverage
measurement useless.

#### Operator overloading

Operator overloading is not used primarily for historical reasons. Instead, method such
as `Plus` are used.

#### Member visibility

Member visibility is not used for implementation hiding. Every member field and function
is `public`.

#### Constructors

Constructors are not used for initialization, chiefly because indicating an error
in a constructor would require throwing an exception, nor does it use constructors for
blanket zero-initialization because of the performance impact of doing this for common
POD classes like `Vector`.

Instances can be zero-initialized using the aggregate-initialization syntax, e.g. `Foo foo = {};`.
This zero-initializes the POD members and default-initializes the non-POD members, generally
being an equivalent of `memset(&foo, 0, sizeof(foo));` but compatible with STL containers.

#### Input- and output-arguments

Functions accepting an input argument take it either by-value (`Vector v`) or
by-const-reference (`const Vector &v`). Generally, passing by-value is safer as the value
cannot be aliased by something else, but passing by-const-reference is faster, as a copy is
eliminated. Small values should always be passed by-value, and otherwise functions that do not
capture pointers into their arguments should take them by-const-reference. Use your judgement.

Functions accepting an output argument always take it by-pointer (`Vector *v`). This makes
it immediately visible at the call site as it is seen that the address is taken. Arguments
are never passed by-reference, except when needed for interoperability with STL, etc.

#### Iteration

`foreach`-style iteration is preferred for both STL and `List`/`IdList` containers as it indicates
intent clearly, as opposed to `for`-style.

#### Const correctness

Functions that do not mutate `this` should be marked as `const`; when iterating a collection
without mutating any of its elements, `for(const Foo &elem : collection)` is preferred to indicate
the intent.

### Coding style

Code is formatted by the following rules:

  * Code is indented using 4 spaces, with no trailing spaces, and lines are wrapped
    at 100 columns;
  * Braces are placed at the end of the line with the declaration or control flow statement;
  * Braces are used with every control flow statement, even if there is only one statement
    in the body;
  * There is no space after control flow keywords (`if`, `while`, etc.);
  * Identifiers are formatted in camel case; variables start with a lowercase letter
    (`exampleVariable`) and functions start with an uppercase letter (`ExampleFunction`).

For example:

```c++
std::string SolveSpace::Dirname(std::string filename) {
    int slash = filename.rfind(PATH_SEP);
    if(slash >= 0) {
        return filename.substr(0, slash);
    }

    return "";
}
```