Ardour Coding Style Guide
That said, this isn't meant to be a strict listing of how your code must be. It is intended to help your code fit in with the rest of the Ardour codebase. This is the style that we have evolved to.
- Any header that is included from a part of the Ardour source tree MUST be included using quotes, not angle brackets. Angle bracket includes are for out-of-tree ('system') includes ONLY.
- List included header files in alphabetical order within the following categories (in this order): C++ headers (e.g. <cstdlib>), C headers (e.g. <sys/time.h>), Boost headers, other non-GUI 3rd party library headers, libpbd headers, libmidi++ headers, GTK headers, gtkmm headers, all others.
- In Python files, use spaces, not tabs. Tabs seem to get a little messed up and spaces are more rigorous where it is important.
- Always use curly braces with if-, for-, do-, and while-statements.
- Since this is C++, use 0, not NULL.
- Similarly, use true/false, not TRUE/FALSE
- If an argument is being passed by reference, and cannot be null, always pass it as a reference, not a pointer.
- Prefer references over pointers
- For core (libardour) data structures, prefer shared_ptr<T> to manage lifetimes
- Never, ever bind a shared_ptr<T> to a sigc::signal. Always convert to a weak_ptr<T> first.
- In GNU Emacs, use the "linux" C and C++ mode style
- Do not use
get_
as part of accessor methods; Exposed member variables should be prefixed with an underscore, with a pair of setter/accessor methods named thusly:type_t _some_member;
type_t some_member() const { return _some_member};
void set_some_member (type_t); - When incrementing any STL iterator, always use
++iter
, notiter++
- When looping over any STL container and performing operations on its contents or iterators to it, always consider the possibility that the operation may remove items from the container and thus invalidate the iterator you are currently operating on. The typical approach looks like this:
for (iter = container.begin(); iter != container.end(); ) { Iterator tmp; tmp = iter; ++tmp; .... do something ... iter = tmp; }
Another approach looks like:for (iter = container.begin(); iter != container.end(); ) { if (... iter matches some condition ...) { iter = container.erase (iter); } else { ++iter; } }
- Because of the previous issue, generally prefer
std::list
overstd::vector
. Erasing an element within a vector invalidates iterators to all later elements. Usevector
only when confident that this issue doesn't exist. - Class types have names like SomeClassType; variables have names like a_pleasant_name.
- Use
const
wherever possible. - Throw
PBD::failed_constructor
to exit constructors abnormally, and catch this exception. - That said, attempt to perform object initialization in an
init
function that is called post-construction. See Meyers for more information on this pattern. - Use cstdio, not stdio.h style standard includes.
- Never declare
using namespace
in a header. - Don't even think of using Microsoft-style "Hungarian" notation
- When constructing paths ("filenames") do not include separators ("/") and try to use
Glib::build_filename()
wherever possible. - Use Glib for any file-system related activities (creating/removing/testing for files)
- Don't write:
if (foo) delete foo;
... C++ handlesdelete (0);
just fine, so justdelete foo
will do. - include header files from a given "source" (e.g. libardour, system C headers, STL headers etc) in alphabetical order wherever possible.
- Always use Tabstops for block-indent (the code must be formatted correctly with "[TAB] = N spaces" for any value of N). Use space only for alignment.
class { [TAB]void example_indent (int first_variable, [TAB][ SPACE TO ALIGN ] int second_variable_that_did_not_fit_in_previous_line) [TAB]{ [TAB][TAB]int count; // variable names are aligned [TAB][TAB]float average; // (some people like it that way) [TAB][TAB]...code... [TAB]} }
The only exception is python code (e.g. wscript), see #3 above. Always use space with a block indentation of 4 spaces. - Avoid trailing whitespace, or trailing tabs on a line, newlines are unix-style line-feed aka ASCII 0x0a (do not use CR+LF). Remove whitespace at end of file.
- Patches are to be submitted as unified diff (e.g.
diff -Naur
orgit diff
, either to the bug-tracker at http://tracker.ardour.org/ or via github https://github.com/Ardour/ardour pull-request (git pulling from similar services with public remote repositories is also fine).
Connecting and Managing Signals
Before Ardour 3.x, we used sigc++ as a "signals + slots" or "anonymous callback" mechanism. But sigc++ is not thread safe and this led to a general lack of thread safety throught out the program. Early in the development of Ardour 3.x, we tried using boost::signals2 until we realized that it was also not thread-safe, and could not be made both thread-safe and realtime acceptable. So Ardour 3.x and later versions use our own implementation of a "signals + slots" mechanism, referred to as PBD::Signal. The following are some guidelines on how to ensure the best use of this API.
There are two major issues to be addressed: thread-safety and connection management.
Thread Safety
to be added
Connection Management
We must ensure if an object makes a connection to a signal, something will be be responsible for disconnecting the connection.
- If the object that creates the connection only makes 1 or 2 connections, add 1 or 2
PBD::ScopedConnection
members to object's class, and assign the return value of the connect call to the relevant member. This ensures that when the object is deleted, the connections will be disconnected (thanks to thescoped_connection
). Example:MIDI::MMC::mmc_connection
orMIDI::Parser::trace_connection
class Foo { Foo(); void method(); ... PBD::ScopedConnection the_connection; } PBD::Signal1<void> aSignal; Foo::Foo () { the_connection = aSignal.connect (boost::bind (&Foo::method, this)); }
- If an object makes more than a couple of connections, it should inherit from
PBD::ScopedConnectionList
and used that class'scoped_connect()
method to make all signal connections. This will also ensure that the connections are disconnected when the object is destroyed.#include "pbd/scoped_connections.h" class Foo : public PBD::ScopedConnectionList { Foo (); void a_method(); } boost::signals2::signal<void()> aSignal; Foo::Foo() { scoped_connect (aSignal, boost::bind (&Foo::method, this)); }
- HOWEVER, if the object makes those connections and then breaks them again without being destroyed (e.g. an object that connects to a set of signals on another object, and drops those connections when told to use a different object), then add a
PBD::ScopedConnectionList*
member to the class. Initialize the pointer to zero; instantiate a new ScopedConnectionList before starting to make connections, and use the list's::add_connection()
method to add the return result of each signal::connect call. To drop all the connections, delete the list object.class Foo { Foo (); ~Foo(); void connect (SomeObjectWithSignals&); ... PBD::ScopedConnectionList* connections; } Foo::Foo() : connections (0) {} Foo::~Foo() { delete connections; } void Foo::connect (SomeObjectWithSignals& obj) { delete connections; connections = new ScopedConnectionList; connections.add_connection (obj.aSignal, boost::bind (.....)); ... }
- Connections to static methods or regular C++ functions do not need to be scoped in these ways. Just connect to the signal normally.