Skip to content

Commit be4e3dc

Browse files
committed
Cleanup, formatting, tool abuse
1 parent c430dae commit be4e3dc

1 file changed

Lines changed: 82 additions & 55 deletions

File tree

README.md

Lines changed: 82 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
- [Naming](#naming)
3636
- [Formatting](#formatting)
3737
- [Use tools](#use-tools)
38+
- [Do not abuse tools](#do-not-abuse-tools)
3839
- [Coding styles](#coding-styles)
3940
- [Follow common styles](#follow-common-styles)
4041
- [Don’t follow styles literally](#dont-follow-styles-literally)
@@ -59,24 +60,30 @@ This is a collection of my passive-aggressive notes on software development,
5960
which are mostly obvious, but have to be repeated time to time. It primarily
6061
concerns C++, Linux, and robotic applications, in other words, embedded /
6162
headless systems consisting of a large number of heterogeneous software
62-
components / services, running with minimal human intervention.
63+
components, running with minimal human intervention.
6364

6465
First principles
6566
================
6667

67-
- Consistency: make your choices and stick to them.
68+
1. Human friendliness: minimize everyone’s mental work.
6869

69-
- Automation: any action that is performed more than once must be automated.
70+
- The less you need to think the more you can do.
7071

71-
- Human friendliness: minimize mental work required to understand things.
72+
2. Consistency: make your choices and stick to them.
73+
74+
- Ensures (1).
75+
76+
3. Automation: any action that is performed more than once must be automated.
77+
78+
- Ensures (1) and (2).
7279

7380
Development environment
7481
=======================
7582

7683
Programming languages
7784
---------------------
7885

79-
- Minimize number of programming languages used in your system, this
86+
- Minimize the number of programming languages used in your system, this
8087
dramatically decreases maintenance costs, facilitates code reviews, improves
8188
code quality, etc.
8289

@@ -101,10 +108,9 @@ Programming languages
101108

102109
- `C++` is the main language for implementation of onboard components. Its
103110
flaws, such as complexity, bad syntax, and lack of fool-proofing, are well
104-
compensated by performance, expressive power, vast amount of development
105-
tools, and reusable open-source libraries. `C++` is also under active
106-
development currently, so it is catching up with new concepts relatively
107-
quickly.
111+
compensated by performance, expressive power, vast amount of development tools
112+
and reusable open-source libraries. `C++` is also under active development
113+
currently, so it is catching up with new concepts relatively quickly.
108114

109115
- `python` is for off-board data processing and analysis, e.g., machine
110116
learning. Sometimes you can use other languages for this purpose, but
@@ -125,7 +131,7 @@ Programming languages
125131

126132
- Young developers often see `make` as a deprecated build tool, which is
127133
wrong -- it is a general purpose automation utility. It was not
128-
superseded by cmake or whatnot.
134+
superseded by cmake or whatnot in this context.
129135

130136
- Don't forget that `/bin/sh` is not the same thing as `/bin/bash`.
131137

@@ -137,7 +143,7 @@ Version control
137143
apply to repository owners as well.
138144

139145
- The main value of reviews is knowledge transfer in the team, both regarding
140-
the software system in development and general programming.
146+
the developed software system and programming in general.
141147

142148
Handling dependencies
143149
---------------------
@@ -147,11 +153,12 @@ Handling dependencies
147153
flaws that directly affect your application.
148154

149155
- If a system package does not exist or unusable, consider other options:
150-
3rd-party package repositories, `vcpkg`, `conan`. However, it may be more
151-
convenient to build a package by yourself to handle dependencies consistently.
156+
3rd-party package repositories, `vcpkg`, `conan`. For new projects `nix` and
157+
`guix` should be considered first. However, it may be more convenient to build
158+
a package by yourself to handle dependencies consistently.
152159

153160
- ROS development is usually performed in workspaces
154-
(http://wiki.ros.org/catkin/workspaces), where you can work with multiple
161+
(<http://wiki.ros.org/catkin/workspaces>), where you can work with multiple
155162
packages coming from various version control systems or tarballs. There exist
156163
several tools for building packages in workspaces taking dependencies into
157164
account: `catkin_make` (consider it to be deprecated), `catkin_tools`,
@@ -160,17 +167,18 @@ Handling dependencies
160167
injection of package meta-information in non-ROS packages to handle
161168
dependencies properly (the process is sometimes referred to as catkinization).
162169
A description of the process can be found at
163-
http://wiki.ros.org/ROS/Tutorials/catkin/CreatingPackage -– it boils down to
170+
<http://wiki.ros.org/ROS/Tutorials/catkin/CreatingPackage> -– it boils down to
164171
adding package description in `package.xml` file and optional special commands
165172
in `CMakeLists.txt`.
166173

167174
- You can also incorporate dependencies into your packages in several ways, all
168175
of which are inferior to workspaces, but may be needed, e.g., to perform
169176
non-invasive catkinization:
170177

171-
- `git` submodules are not too bad, but depend on external repositories which
172-
makes them fragile. Also, they are not handled by git transparently, so
173-
you have to remember to do recursive fetch, etc.
178+
- `git` submodules are not too bad, but depend on external repositories
179+
which makes them fragile and difficult to fork. Also, they are not
180+
handled by git transparently, so you have to remember to do recursive
181+
fetch, etc.
174182

175183
- `cmake` external projects should never be used -– in addition to being
176184
fragile as git submodules, they are difficult to be used in the right way
@@ -190,7 +198,7 @@ Continuous integration
190198
compilation. testing, static/dynamic analysis, binary package generation, etc.
191199
Hence, you should implement a development environment which supports those
192200
operations and then build CI based on it. A notable example is
193-
https://github.com/asherikov/ccws.
201+
<https://github.com/asherikov/ccws>.
194202

195203
- Do not use CI pipelines for scripting -– all essential functionality must be
196204
performed in a generic scripting language to facilitate migration between
@@ -235,14 +243,19 @@ shared by many developers. It is, however, even more inconvenient to perform
235243
binary package releases for this purpose –- the ROS buildfarm system is not
236244
designed for this, which is, in my opinion, a direct consequence of treating
237245
this task as non-interactive and “pure-CI”. I’ve tried to address it in
238-
https://github.com/asherikov/ccws by allowing developers to generate binary
246+
<https://github.com/asherikov/ccws> by allowing developers to generate binary
239247
packages locally.
240248

241249
Documentation
242250
-------------
243251

244252
- Document your classes, methods, and source files using doxygen
245-
http://www.doxygen.org/.
253+
<http://www.doxygen.org/>. There are a some alternatives, e.g.
254+
<https://github.com/cppalliance/mrdocs>,
255+
<https://github.com/NaturalDocs/NaturalDocs>, <https://github.com/hdoc/hdoc>,
256+
<https://github.com/vovkos/doxyrest>,
257+
<https://github.com/copperspice/doxypress>, but they do not seem to be
258+
significantly better than doxygen atm.
246259

247260
- Each repository must contain a README.md file with a brief description of its
248261
purpose.
@@ -255,8 +268,8 @@ Documentation
255268
text in general rather than graphics. Moreover, it is more difficult to keep
256269
design diagrams in sync with implementation. For these reasons, I prefer tools
257270
that extract information from the code, e.g., doxygen. Another interesting
258-
example is https://github.com/boost-ext/sml which allows generation of finite
259-
state machine diagrams from their C++ implementations.
271+
example is <https://github.com/boost-ext/sml> which allows generation of
272+
finite state machine diagrams from their C++ implementations.
260273

261274
Runtime failures
262275
================
@@ -265,7 +278,7 @@ Out of memory (OOM)
265278
-------------------
266279

267280
- Linux (at least Ubuntu) does not handle OOM well by default
268-
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/159356, so you have to
281+
<https://bugs.launchpad.net/ubuntu/+source/linux/+bug/159356>, so you have to
269282
take extra measures to avoid such situations and protect your system from
270283
freezes.
271284

@@ -333,7 +346,7 @@ Configuration-driven development
333346
inferior to `YAML`/`JSON`.
334347

335348
- Use serialization / reflection libraries to abstract from a particular file
336-
format, e.g., https://github.com/asherikov/ariles.
349+
format, e.g., <https://github.com/asherikov/ariles>.
337350

338351
- Don’t forget to respect transformation API symmetry –- sooner or later you are
339352
going to need to modify configuration during execution and export it for
@@ -359,8 +372,9 @@ Let it crash
359372
------------
360373

361374
“Let it crash” approach to failure handling comes from Erlang – a language
362-
designed for telecommunication applications
363-
https://en.wikipedia.org/wiki/Erlang\_(programming_language)#%22Let_it_crash%22_coding_style
375+
designed for telecommunication applications <a
376+
href="https://en.wikipedia.org/wiki/Erlang\_(programming_language)#%22Let_it_crash%22_coding_style"
377+
class="uri">https://en.wikipedia.org/wiki/Erlang\_(programming_language)#%22Let_it_crash%22_coding_style</a>
364378

365379
You have to accept that your programs are going to crash, which means that:
366380

@@ -401,7 +415,7 @@ State machines
401415
State machines are often employed for representing robot behaviors, but in my
402416
opinion they should be used more for implementation of individual services. Any
403417
time you work on a service that changes its behavior in response to some command
404-
messages, for example using http://wiki.ros.org/actionlib, it is necessary to
418+
messages, for example using <http://wiki.ros.org/actionlib>, it is necessary to
405419
consider a finite state machine.
406420

407421
Project bootstrapping
@@ -449,7 +463,7 @@ Performance
449463

450464
- Performance optimization is often focused on computational complexity of
451465
algorithms, i.e., the mount of resources required to run them
452-
(https://en.wikipedia.org/wiki/Computational_complexity). In practice, it is
466+
(<https://en.wikipedia.org/wiki/Computational_complexity>). In practice, it is
453467
usually a bad approach when you work with non-trivial data: the type of
454468
resources and access to them are much more important. Pay attention to memory
455469
access and especially I/O. For example, loading a geographic model from a file
@@ -459,24 +473,25 @@ Performance
459473
- Legacy algorithms often measure their complexity in number of single floating
460474
point operations. Modern hardware is actually much better at performing
461475
arithmetic operations in bulk due to vectorization instructions, e.g., see
462-
https://eigen.tuxfamily.org/index.php?title=FAQ#Vectorization. For this
476+
<https://eigen.tuxfamily.org/index.php?title=FAQ#Vectorization>. For this
463477
reason, brute-force algorithms that use plain linear algebra may perform
464478
better than classic algorithms containing loops, conditionals, and recursion.
465479
Note that interpreted languages, such as `Matlab` and `python`, can also
466480
benefit from matrix-based operations for slightly different reasons
467-
https://www.mathworks.com/help/matlab/matlab_prog/vectorization.html.
481+
<https://www.mathworks.com/help/matlab/matlab_prog/vectorization.html>.
468482

469483
Volumetric data
470484
---------------
471485

472-
OcTree (https://octomap.github.io/) is commonly used for representation of
486+
OcTree (<https://octomap.github.io/>) is commonly used for representation of
473487
volumetric data, but it is not always a good solution:
474488

475489
- when you build a map based on readings from range sensors such as lidars don’t
476490
expect to benefit from tree pruning of occupied cells -– the scans give you a
477491
thin surface of objects, so the tree leafs cannot be merged together;
478492

479-
- in terms of performance OcTree is inferior to VDB (https://www.openvdb.org/);
493+
- in terms of performance OcTree is inferior to VDB
494+
(<https://www.openvdb.org/>);
480495

481496
- OcTrees, however, are useful when you need to work with different resolutions
482497
of the same map, this structure naturally supports such slicing.
@@ -554,8 +569,8 @@ Date, time, and locale
554569
automatically formatted during I/O in accordance with system locale. For
555570
example, French locale uses comma to separate decimal part of floating point
556571
numbers instead of dot, which leads to funky issues like this
557-
https://github.com/zeux/pugixml/issues/469. To be on a safe side, enforce `C`
558-
(`POSIX`) locale in deployment and while performing formatted I/O.
572+
<https://github.com/zeux/pugixml/issues/469>. To be on a safe side, enforce
573+
`C` (`POSIX`) locale in deployment and while performing formatted I/O.
559574

560575
General style policies
561576
======================
@@ -591,7 +606,7 @@ Naming
591606
`namespace logger { class LoggerParameters; }`. This repetition is redundant
592607
and should be avoided: `namespace logger { class Parameters; }`. Another
593608
example is `ROS` convention of subdirectory naming in robot description
594-
repositories, e.g., https://github.com/ros-naoqi/pepper_robot, where each
609+
repositories, e.g., <https://github.com/ros-naoqi/pepper_robot>, where each
595610
subdirectory includes redundant robot name. Yes, the reason is to match
596611
directory and package names, but practical value of this convention is next to
597612
zero.
@@ -651,9 +666,18 @@ Use tools
651666
warnings as errors, otherwise they are useless. There can be exceptions for
652667
specific warnings of course. Counterarguments like “-Werror Introduces a
653668
Toolchain Dependency”
654-
(https://embeddedartistry.com/blog/2017/05/22/werror-is-not-your-friend/) are
655-
weak, since the real problem there is that the language standard is not
656-
enforced as well.
669+
(<https://embeddedartistry.com/blog/2017/05/22/werror-is-not-your-friend/>)
670+
are weak, instead of dropping `-Werror` enforce language standard and test
671+
with all supported toolchains.
672+
673+
Do not abuse tools
674+
------------------
675+
676+
- LLM-based code auto-completion tools, such a `GitHub Copilot` are quite good
677+
at generating boilerplate comments. The value of such comments, however, is
678+
often next to zero. Do not pollute your sources with comments like
679+
`/* vector of integers */`, they are not only useless and distracting, but may
680+
also be misleading if they get out of sync with the code.
657681

658682
Coding styles
659683
=============
@@ -663,9 +687,11 @@ Follow common styles
663687

664688
### C++
665689

666-
- https://google.github.io/styleguide/cppguide.html
667-
- http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines
668-
- http://www.stroustrup.com/JSF-AV-rules.pdf
690+
- <https://google.github.io/styleguide/cppguide.html>
691+
- <http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines>
692+
- <http://www.stroustrup.com/JSF-AV-rules.pdf>
693+
- <https://github.com/janwilmans/guidelines>
694+
- <https://github.com/cpp-best-practices/cppbestpractices>
669695

670696
Don’t follow styles literally
671697
-----------------------------
@@ -738,7 +764,8 @@ C++
738764
values –- always use an enumeration in such cases.
739765

740766
- All enumerations must be defined within some container class scope. It is
741-
recommended to use some wrappers, e.g, http://aantron.github.io/better-enums/.
767+
recommended to use some wrappers, e.g,
768+
<http://aantron.github.io/better-enums/>.
742769

743770
- The general rule for handling enumerations is to use a switch: known values
744771
should be handled as needed, all unknown values must be captured by `default`
@@ -778,7 +805,7 @@ C++
778805

779806
- Input parameters must be passed by reference unless their type is fundamental:
780807
integral, floating point, or void, see
781-
https://en.cppreference.com/w/cpp/language/types.
808+
<https://en.cppreference.com/w/cpp/language/types>.
782809

783810
- Output parameters must be gathered at the end of the parameter list:
784811
`doSomething(input1, output1, output2, input2 = <default_value>)`.
@@ -821,8 +848,8 @@ C++
821848
member variables on declaration and in member initializer lists of
822849
constructors when possible, which is obviously not always the case. When such
823850
conventions are enforced you end up with member initialization logic scattered
824-
all over the place. In my opinion a dedicated initializaion method called from
825-
a constructor is the most transparent approach.
851+
all over the place. In my opinion a dedicated initialization method called
852+
from a constructor is the most transparent approach.
826853

827854
### Macro
828855

@@ -923,11 +950,11 @@ function returns a product between two different matrices generated on spot.
923950
Such errors cannot be detected by compiler or sanitizers, since the code is 100%
924951
correct. They are also difficult to pick up by reading the code – you have to
925952
know how Eigen API works and what to look for. Even though the issue is well
926-
known and documented https://eigen.tuxfamily.org/dox/TopicPitfalls.html#title3
953+
known and documented <https://eigen.tuxfamily.org/dox/TopicPitfalls.html#title3>
927954
developers following the ‘modern’ style fall for it over and over, e.g.
928955

929-
- https://stackoverflow.com/questions/59586537/eigen-gives-wrong-result-when-not-storing-intermediate-result
930-
- https://stackoverflow.com/questions/55962829/eigen-c-how-can-i-fixed-the-values-after-a-random-matrix-initialization
956+
- <https://stackoverflow.com/questions/59586537/eigen-gives-wrong-result-when-not-storing-intermediate-result>
957+
- <https://stackoverflow.com/questions/55962829/eigen-c-how-can-i-fixed-the-values-after-a-random-matrix-initialization>
931958

932959
Another possible side-effect of using `auto` with `Eigen` is performance
933960
degradation: `auto mat3 = mat2 * mat1` here `mat3` is an expression rather than
@@ -948,7 +975,7 @@ wrong solution to this problem:
948975
This brings us to another important point: type is documentation which is
949976
automatically verified and enforced by compiler. Type omission makes the code
950977
more difficult to comprehend, e.g., consider an example from
951-
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines
978+
<http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines>
952979

953980
auto hello = "Hello!"s; // a std::string
954981
auto world = "world"; // a C-style string
@@ -975,13 +1002,13 @@ Naming
9751002
possible that that would be necessary for disambiguation?
9761003

9771004
- ROS package naming conventions are a good starting point
978-
http://www.ros.org/reps/rep-0144.html.
1005+
<http://www.ros.org/reps/rep-0144.html>.
9791006

9801007
Layout
9811008
------
9821009

9831010
- ROS1 catkin package template example
984-
https://github.com/asherikov/ccws/tree/master/pkg_template/catkin
1011+
<https://github.com/asherikov/ccws/tree/master/pkg_template/catkin>
9851012

9861013
- Public headers must be located in `include/mypackage/` subfolder. Non-public
9871014
headers should be kept together with source files in `src`.
@@ -1017,7 +1044,7 @@ mind:
10171044
- Networking stack takes measures to prevent conflicts between TCP sessions, in
10181045
particular a side that initiated session termination blocks corresponding
10191046
socket in `TIME_WAIT` state
1020-
https://serverframework.com/asynchronousevents/2011/01/time-wait-and-its-design-implications-for-protocols-and-scalable-servers.html
1047+
<https://serverframework.com/asynchronousevents/2011/01/time-wait-and-its-design-implications-for-protocols-and-scalable-servers.html>
10211048
Default timeout on Linux for this state is 60 second, i.e., under certain
10221049
circumstances you won’t be able to reestablish a TCP connection for a whole
10231050
minute, which is more than enough to be fatal in robotic applications.
@@ -1032,7 +1059,7 @@ Inertia
10321059
Ironically some commercial CAD systems incorrectly export inertia matrices of
10331060
rigid bodies to `URDF`, so it is a good idea to verify them. One way to achieve
10341061
this is to perform eigendecomposition of the matrix
1035-
https://en.wikipedia.org/wiki/Moment_of_inertia#Principal_axes to obtain
1062+
<https://en.wikipedia.org/wiki/Moment_of_inertia#Principal_axes> to obtain
10361063
principal axes and moments of inertia, which can be used to specify orientation
10371064
and extent of a rectangular cuboid. Obtained cuboid should roughly match visual
10381065
representation of a rigid body.
@@ -1044,7 +1071,7 @@ ROS
10441071
---
10451072

10461073
- Try to follow standard ROS conventions listed at
1047-
http://www.ros.org/reps/rep-0000.html
1074+
<http://www.ros.org/reps/rep-0000.html>
10481075

10491076
- Avoid scripting in launch files: `if` and `unless` attributes, python code
10501077
injection.

0 commit comments

Comments
 (0)