12 Jan
2015
12 Jan
'15
2:48 a.m.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
<big snip>
Yes, but an operator is more visible than a missing brace :-)
Forcing parenthesis everywhere can make code more difficult to read sometimes, as in:
if ((a == 3) || (b == 5) || (c == 4))
vs
if (a == 3 || b == 5 || c == 4)
So I also favor the "best legibility" rule. Let's restate it as something
like "the minimum number of parenthesis that
clearly isolate a conditional logic block. Example:
Right: if (a == 3 || (b == 5 && d != 20) || c == 4)
Wrong: if (a == 3 || b == 5 && d != 20 || c == 4)
If we use this style, for consistency I think the constructor of a derived class should
be:
MyClass::MyClass(int i, char c, int *p) :
TQObject(),
m_i(i),
m_c(c),
m_p(p),
m_d(d)
{
do_something();
}
since the "base block" is just another part of the class object, as any other
member.
My only concern is that with big classes, the list of members can be quite long and so
requires some scrolling from
the constructor definition line to the actual constructor body. I usually use this style:
MyClass::MyClass(int i, char c, int *p) :
m_i(i), m_c(c), m_p(p),
m_d(d)
{
do_something();
}
which is somehow more compact, but I am fine with both versions.
This is of
course subjective and not a big deal for me as well (I have already worked with both style
actually).
Mostly two reasons: 1) the operator at the end of the line tells me immediately that the
conditional expression
is not over and I need to continue reading the next line
Wouldn't the lack of a curly brace state the same thing, given our rules on curly
braces? 2) logically easier to read complex statement.
For example
if (a == 3 && (b == 5 || c == 4 || (d ==
10 && e == 20)))
rather than:
if (a == 3 && (b == 5 || c == 4 || (d ==
10 && e == 20)))
I find the second one more prone to
misinterpretation, since the || in the 3rd row might look at the same level
as the && in the second row at first sight. And in TDE I have seen some rather
complex and long ifs :-) Just my 2
cents, let's see what Slavek thinks as well.
Point taken. However, I think we need to add a rule as well that states all conditionals
must be enclosed in
parenthesis; I have cleaned up so many compiler warnings and so much buggy code becase of
lazy programmers in this
respect that I don't want to see another unenclosed conditional. ;-)
If this rule is enforced, your second example becomes: if ((a == 3) && ((b == 5)
|| (c == 4) || ((d == 10) && (e ==
20))))
which is a bit easier to read, but overall this style is still harder to read than your
first example when more
than one condition is present on one line. Perhaps we should allow both styles and
simply state that the style
providing "best legibility" should be used?
The number of long/complex ifs in the codebase are why I am insisting this be hammered
out properly. :-) We
haven't head from Slavek in a while so I guess we'll keep drawing up the
specification and he can review it and
comment when he has time.
Another thing
is class/struct member names. I usually add an "m_" to every member, so I know
immediately that a
variable is a class member. TDE is a very wide variety of class member names. What is
your opinion?
Yes! This is Hungarian notation for member variable scope, and should be strongly
enforced. Added to spec for new
code, though I don't think we can go back and fix this automatically.
Another thing is class constructor parameter
definition. Which one is better?
<style snip>
I prefer this: MyClass::MyClass(int i, char c, int *p) : m_i(i), m_c(c), m_p(p), m_d(d) {
do_something(); }
and where a derived class is being created: MyClass::MyClass(int i, char c, int *p) :
TQObject(), m_i(i), m_c(c),
m_p(p), m_d(d) { do_something(); }
Is this acceptable?
I have also added a question to the Etherpad regarding loop control statement formatting;
if you could respond to
that I'd appreciate it.
Thanks!
Done. Also added a question about variable/function/class names.
Cheers
Michele
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
iQIcBAEBCgAGBQJUszYFAAoJECp1t8qK3tXPMoEQAI9DM6le/SyBO6Wte+WGHtS+
oR5Qh1ndbVT+B+5U808T4l4ARdd2wNPdNxFWeTQp3XfdSz/KdKNccv3NZZcP3nPO
NZcWVnHnCOhRfzYAg1g9KhRtu/0LZjSZrIVubm9NtOpucuTvU6NjuX3jT9PGt7GY
mmmCYYxgBKl1fyAFY8oKNWVJyvW7wWR0+W7OW4UExs2d4yajVTVXTqnYv6iOH35G
FPiYwU3fc3ACUs6LKNIycALVOwE4PrF0Mae2ar3SFjw9J3bbY3Rbqz7ZSrFuk9Zq
BPxxNcSITlay1c0LWDudShyDEYc2FV9xosLAHvuX0R3daGxTCSynIOVmi/HMlIwe
lKDS3p91fmOJClk0f6Kct+8h7dM5td8jt9GQeZAC6PPW55AC0rHD6PKZudb0ZRDg
Q7OQrkv+YPv/kg4DJl+duOArr4L4V688cR9QGRR0l1A/oX1J6JPX+6h0mP6jm2kL
kiEAHeyskp/3uRiD+UgkPyiGNMM/wxHbhxKWYe241IuBTsVq9Uwcy7uT5Ah1cUlE
mEP0SW0WO26gtwLNnoSuX5ZfkTFootmbzyntLiW0Qag3uEBSh5HPg1X2eq/s39P5
4QgtCsh1Q7pyCJazsLPGzZOwRXVdl2eXi+dxdOu65KzAYbOkgzc8YF4mlKJrBN6s
59ijrEkXJt0KWCNdWFBD
=1ZlX
-----END PGP SIGNATURE-----