Reverses some code style changes introduced by A. Nordmann
The fillowing huge patch (more than 300 deletion !!!) Reverse some
code style change previously introduced. Most of teh changes
concerned are about indenting, and to follow a stupid rules that a
line should always *whatsoever* be under 80 char. If this is the goal
to pursue I will advise the author to change to abother language than
C++ and avoid Self - Documenting code (which I consider a good
practice) as it could be really too verbose to follow that
rules. However it is largely accepted that breaking long and complex
statement on several meaningfull line is a good practice.
Some of the changes introduced were really making the code base ambiguous, like notably :
sbcp::amarsi::MotorDriver::Ptr res = d_bus->OpenDevice
< sbcp::amarsi::MotorDriver > (def.BoardID());/*diff of breaked template parameter */This can fool the quick reader into recognizing a comparison instead
of a template. Yes it comes from the fact that the C++ syntax is a
poor choice in first place.
Another changes which seems bad to me is :
-BrushlessParameterSection::BrushlessParameterSection(options::Section::Ptr s)
- : options::ConfigSection(s)
- , d_hPGain(Section())
- , d_hIGain(Section())
- , d_hDGain(Section())
- , d_hStiffness(Section())
- , d_hDamping(Section())
- , d_hPreCompression(Section())
- , d_hMaxSpeed(Section())
- , d_hMaxAcceleration(Section())
- , d_hMaxTorque(Section()){
+BrushlessParameterSection::BrushlessParameterSection(options::Section::Ptr s) :
+ options::ConfigSection(s), d_hPGain(Section()), d_hIGain(Section()), d_hDGain(
+ Section()), d_hStiffness(Section()), d_hDamping(Section()), d_hPreCompression(
+ Section()), d_hMaxSpeed(Section()), d_hMaxAcceleration(
+ Section()), d_hMaxTorque(Section()) {(NDLA: yes woth weird tab settings it renders like this in some text editors)
I agree that the syntax
class foo
: bar()
, baz()
...
, barbar() {
}Is rather personal, but it tries to follow the given rules :
- Only one declaration per line. This is a idiom found in most good
- code style Clearly isolate the initilaization section (where order does matter a lot) of the constructor body
Especially, the following syntax is really bad
class foo:
field1(),...,field2() {I would not mind changing my habits, if the two above point
are carefully respected.
Another pattern that I found in the changes and is awful is this one during class declaration :
class foo: public bar {
};Note the absence of space between ':' and the declared class
name. With really long and self-documenting class name, this ':' is
hard to catch at first glance, especially with no syntax
coloring. This is really minor but please consider adding a space
here.
And finally one of the most anoying changes are with alignement, and
poorly scripted / configured automatic code style formatter
(expecially the stupid eclipse one, that takes 2 hours to configure
properly). Here is some example, that makes the code as much readable
than brainfuck (wikipedia link).
- if(d_isHip){
- hwDownJointAngle = (d_isReversed? -1: 1) * userDownJointAngle / (2.0 * M_PI) * 4096.0 + d_motorPositionLimit / 2.0;
- }
- else{
- hwDownJointAngle = d_isReversed ? ( d_motorPositionLimit * (1.0 - userDownJointAngle ) )
- : ( d_motorPositionLimit * userDownJointAngle );
+ if (d_isHip) {
+ hwDownJointAngle = (d_isReversed ? -1 : 1) * userDownJointAngle
+ / (2.0 * M_PI) * 4096.0 + d_motorPositionLimit / 2.0;
+ } else {
+ hwDownJointAngle =
+ d_isReversed ?
+ (d_motorPositionLimit * userDownJointAngle):
+ (d_motorPositionLimit * (1.0 - userDownJointAngle));
}Where here the structure of the ternary (which is the top operator)
was underlined by the first indentation and is totatly obscured by the
automatic eclipse correction.
Or another example :
- d_motordrivers[rci::oncilla::LEFT_FORE] = OpenAndConfigureMotorDriver(devices.LeftFore(), - motorConfig, - timestep); - d_motordrivers[rci::oncilla::RIGHT_FORE] = OpenAndConfigureMotorDriver(devices.RightFore(), - motorConfig, - timestep); - d_motordrivers[rci::oncilla::LEFT_HIND] = OpenAndConfigureMotorDriver(devices.LeftHind(), - motorConfig, - timestep); - d_motordrivers[rci::oncilla::RIGHT_HIND] = OpenAndConfigureMotorDriver(devices.RightHind(), - motorConfig, - timestep); + d_motordrivers[rci::oncilla::LEFT_FORE] = OpenAndConfigureMotorDriver( + devices.LeftFore(), motorConfig, timestep); + d_motordrivers[rci::oncilla::RIGHT_FORE] = OpenAndConfigureMotorDriver( + devices.RightFore(), motorConfig, timestep); + d_motordrivers[rci::oncilla::LEFT_HIND] = OpenAndConfigureMotorDriver( + devices.LeftHind(), motorConfig, timestep); + d_motordrivers[rci::oncilla::RIGHT_HIND] = OpenAndConfigureMotorDriver( + devices.RightHind(), motorConfig, timestep);
Where a line break is inserted right after the roght parenthesis of a
function / method call, which makes a really weird line of comma -
separated statement.
Finally, after rewriting, most of the Code style changes that were
introduced are missing space between parenthesis and comma. Maybe it
would be better in order not to defigurate work of others to stop
relying on automatic code stylee scripts, and use sed. Here is the two
cli lines that would have saved me 1h30 of work and for you this long
email.
bash
sed -i "s/){/) {/g" *.h *.cpp
sed -i "s/,\(\W\)/, \1/g" *.h *.cpp`