User Tools

Site Tools


coding_guidelines

Differences

This shows you the differences between two versions of the page.

Link to this comparison view

Both sides previous revision Previous revision
Next revision
Previous revision
Next revision Both sides next revision
coding_guidelines [2019/05/10 17:55]
tapir Remove unnecessary advice, it simply doesn't matter
coding_guidelines [2020/02/05 16:46]
tapir [Line Widths] Broken lines indent
Line 15: Line 15:
 Since 2019-05-09 our code formatting rules are defined by the [[https://​github.com/​mixxxdj/​mixxx/​blob/​master/​.clang-format|.clang-format]] configuration file. It can be found in the project root directory and contains settings for [[https://​clang.llvm.org/​docs/​ClangFormat.html|ClangFormat]]. **Make use of it by auto-formatting new or modified code segments as soon as you need to touch it!** Since 2019-05-09 our code formatting rules are defined by the [[https://​github.com/​mixxxdj/​mixxx/​blob/​master/​.clang-format|.clang-format]] configuration file. It can be found in the project root directory and contains settings for [[https://​clang.llvm.org/​docs/​ClangFormat.html|ClangFormat]]. **Make use of it by auto-formatting new or modified code segments as soon as you need to touch it!**
  
-The following options are available for auto-formatting the code. +ClangFormat ​version >= 4.0 is required.
- +
-clang-format ​version >= 4.0 is required+
  
 On Ubuntu, you can install clang format via. On Ubuntu, you can install clang format via.
Line 35: Line 33:
  
 [[https://​dx13.co.uk/​articles/​2015/​4/​3/​Setting-up-git-clang-format.html|Setting-up git-clang-format]] [[https://​dx13.co.uk/​articles/​2015/​4/​3/​Setting-up-git-clang-format.html|Setting-up git-clang-format]]
 +
 +You can for example do this after each commit during development: ​
 +<code bash>
 +git-clang-format HEAD^1
 +git commit -a --amend
 +</​code>​
 +
 +or if the PR is already under review: ​
 +<code bash>
 +git-clang-format upstream/​master ​
 +git commit -a -m"​apply git-clang-format changes"​
 +</​code>​
 + 
  
 Apply //​clang-format//​ to individual source files (only permitted for new files): Apply //​clang-format//​ to individual source files (only permitted for new files):
  
-''​clang-format -style=file [<​file>​ ...]''​+''​clang-format ​-i -style=file [<​file>​ ...]''​
  
 //​clang-format//​ will pick up the //​.clang-format//​ file from the current or any parent directory of the source file(s). //​clang-format//​ will pick up the //​.clang-format//​ file from the current or any parent directory of the source file(s).
Line 75: Line 86:
 ===== Line Widths ===== ===== Line Widths =====
  
-Please configure your editor to have a max column-width of 80-columns. While it is not a strict requirement,​ 80-column cleanliness makes it easy to tile multiple buffers of code across a laptop screen, which provides significant efficiency gains to developers. Use double indent (8-spaces) for broken lines or align with the opening "​("​ in the line above for hanging indents.+Please configure your editor to have a max column-width of 80-columns. While it is not a strict requirement,​ 80-column cleanliness makes it easy to tile multiple buffers of code across a laptop screen, which provides significant efficiency gains to developers. 
 + 
 +For Mixxx'​s clang-format compatibility (ColumnLimit:​ 0):  
 +  * Use double indent (8-spaces) for broken lines 
 +  * Break line after binary operators.  
 +  * If you break a list of function parameters, put each parameter on a single ​line.  
  
  
Line 461: Line 477:
     * ''​parented_ptr''​ for ownership by the QT object tree: e.g. ''​auto p = make_parented<​T>​(...)''​     * ''​parented_ptr''​ for ownership by the QT object tree: e.g. ''​auto p = make_parented<​T>​(...)''​
  
-  * Pass raw pointer ''​T*''​ if no ownership is transfered. A function which receives a raw pointer should not hold onto the pointer ​outside the scope of the function (e.g. by storing it in a member variable). An exception is the parent pointer passed into the constructor of a ''​QObject''​ and internally stored as a non owning ​reference. Do not pass smart pointers by reference. ​+  * Pass reference ''​const T&''​ or ''​T&''​ if no ownership is transfered. ​Pass raw pointer ​''​const T*''​ or ''​T*''​ if no ownership is transfered ​and ''​nullptr''​ is a valid argument value. A function which receives a reference or raw pointer should not hold onto it outside the scope of the function (e.g. by storing it in a member variable). An exception is the parent pointer passed into the constructor of a ''​QObject''​ and internally stored as a non owning reference.
  
-  * ''​parented_ptr''​ requires the parent to be passed into the constructor at the time of creation. If a pointer that will eventually be managed ​by the QT object tree needs to be temporarily created without a parent, first create the object ​''​p'' ​with ''​std::​make_unique''​, and then later create a new variable using ''​parented_ptr(p.release())''​ once the underlying object gets a parent.+  * Pass smart pointers ​by value if ownership is transfered (''​std::​unique_ptr''​) or shared (''​std::​shared_ptr''​)Do not pass smart pointers by reference.
  
-  * Never store a ''​parented_ptr'' ​in an object ​that outlives the parent. If the lifetime of the child relative to the parent ​is not clear then store the output of ''​parented_ptr::​toWeakRef()'' ​instead.+  * Use ''​make_parented''​ to create objects derived from ''​QObject''​ that will be assigned to a parent ​(and will therefore be managed by the Qt object tree). The created object must get a parent ​before ​the ''​parented_ptr'' ​is destructedExample:
  
-  ​Any exceptions ​to these guidelines must be clearly documented inline in the code+    auto pBrowseButton = make_parented<​QPushButton>​(tr("​Browse"​));​ 
 +    // *pBrowseButton is not assigned ​to a parent yet 
 +    pLayout->​addWidget(pBrowseButton);​ 
 +    // Now *pLayout is the parent
  
 +  * The destructor of ''​parented_ptr''​ asserts that the pointed-to object actually has a parent. This ensures that the pointed-to object isn't leaked. A consequence is that **a ''​parented_ptr''​ must never dangle**. Never store a ''​parented_ptr''​ in an object that outlives the parent. This is most easily done by storing the ''​parented_ptr''​ inside exact the parent. If the lifetime of the pointer relative to the parent is not clear then store the output of ''​parented_ptr::​toWeakRef()''​ instead of a ''​parented_ptr''​.
 +
 +  * Any exceptions to these guidelines must be clearly documented inline in the code.
  
 ===== Assertions ===== ===== Assertions =====
Line 493: Line 515:
 </​code>​ </​code>​
  
-               +===== QString =====
  
 +Use [[http://​doc.qt.io/​qt-5/​qstring.html#​QStringLiteral|QStringLiteral]]. This has a variety of [[https://​woboq.com/​blog/​qstringliteral.html|benefits]]. ​
  
 +Escape non ASCII characters: ​
  
 +<code cpp-qt>
 +QStringLiteral("​Hello I\u2019ve to go");
 +</​code>​
  
 +QChars can be initialized with ASCII characters 16 bit Unicode L’\u00fc'​ if required. Both are constexpr. ​
  
 +<code cpp-qt>
 +constexpr QChar kc = '​c'​
 +constexpr QChar kue = L'​\u00fc'​ // for "​ü"​
 +</​code>​
  
 +From Mixxx 2.3 we set QT_USE_QSTRINGBUILDER to use QStringBuilder for operator+. Use + in favour of % for better readability. ​         ​
  
  
Line 505: Line 538:
  
  
-===== C++11 ===== 
  
-As of the Mixxx 2.release, Mixxx is switching to C++11. We are taking a conservative approach to adopting C++11 features and whitelisting them one by one. If a C++11 feature you would like to use is not listed here, please email mixxx-devel to make a case for it and we will consider whitelisting it.+ 
 + 
 + 
 + 
 + 
 +===== C++17 ===== 
 + 
 +As of the Mixxx 2.release, Mixxx is switching to C++17. We are taking a conservative approach to adopting C++11/​14/​17 ​features and whitelisting them one by one. If a C++11/​14/​17 ​feature you would like to use is not listed here, please email mixxx-devel to make a case for it and we will consider whitelisting it.
  
 We are limited to what is supported across our 3 supported compilers: We are limited to what is supported across our 3 supported compilers:
-  * [[http://​clang.llvm.org/​cxx_status.html|Clang 3.3]] +  * [[http://​clang.llvm.org/​cxx_status.html|Clang ​>= 3.4]] 
-  * [[https://​gcc.gnu.org/​projects/​cxx0x.html|GCC ​4.8]] +  * [[https://​gcc.gnu.org/​projects/​cxx0x.html|GCC ​>= 5.4 (Ubuntu Xenial)]] 
-  * [[https://​msdn.microsoft.com/​en-us/​library/​hh567368.aspx|Microsoft Visual Studio ​2015 Update 3]]+  * [[https://​msdn.microsoft.com/​en-us/​library/​hh567368.aspx|Microsoft Visual Studio ​2017 (_MSC_VER >= 1910)]]
  
 In general, Microsoft Visual Studio is the one that prevents us from using features. In general, Microsoft Visual Studio is the one that prevents us from using features.
Line 564: Line 603:
 Use. Use.
  
-==== unicode string literals ==== 
  
-**Do Not Use**. Once we switch to Qt5, you should use [[http://​doc.qt.io/​qt-5/​qstring.html#​QStringLiteral|QStringLiteral]]. This has a variety of [[https://​woboq.com/​blog/​qstringliteral.html|benefits]]. ​ 
  
 ==== right angle brackets ==== ==== right angle brackets ====
Line 580: Line 617:
 In contrast to the pre-C++11 rules (see above) when using //​override//​ on a function in a derived class it is recommended to omit the redundant //virtual// keyword, because //​override//​ implies //​virtual//​. In contrast to the pre-C++11 rules (see above) when using //​override//​ on a function in a derived class it is recommended to omit the redundant //virtual// keyword, because //​override//​ implies //​virtual//​.
  
-Destructors in derived classes should also be marked with //​override//​ instead of //​virtual//​. This ensures at compile time that the base class has declared a //virtual// destructor. If the base class has not declared a //virtual// destructor the destructor of a derived class might not be invoked.+Destructors in derived classes should also be marked with //​override//​ instead of //​virtual//​. This ensures at compile time that the base class has declared a //virtual// destructor. If the base class has not declared a //virtual// destructor the destructor of a derived class might not be invoked. This applies also for default destructors even if it looks noisy
  
 +<code cpp-qt>
 +~ClassName() override = default;
 +</​code>​
 ==== alignment ==== ==== alignment ====
  
coding_guidelines.txt · Last modified: 2020/05/22 06:30 by xerus