12 Jan
2015
12 Jan
'15
3:25 p.m.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA224
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.
But the base class(es) are fundamentally different than the other members;
I like the visual distinction on first glance. Slavek, what is your
opinion here?
-----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)
The problem with allowing these is that if I need to refactor the code to
add in a single conditional I need to add parenthesis, which a.) is an
additional source of error and b.) messes up the difference tracking
making it hard for other developers to see what the functional change was.
I think I'm going to override you on this one and force the parenthesis.
;-)
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? 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.
While that is more compact, it introduces a problem with maintainability.
Say I need to add a new member variable, strongly associated with m_c.
That changes the definition to:
MyClass::MyClass(int i, char c, int *p) :
m_i(i), m_c(c), m_cRel(c_r),
m_p(p), m_d(d)
{
do_something();
}
which a.) required quite a bit of editing (enough to make me think twice
about adding the new member variable!) and b.) now shows two lines as
completely changed in the difference output. The less-compact style makes
it easy to add the variable, and shows at a glance what changed in the
difference output.
Tim
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
iFYEARELAAYFAlS0V8YACgkQLaxZSoRZrGHgHwDdHiwFCiiuQIhoVFkCRMb31+KJ
5BX8UEpWkZhniADgq5Chml0ywuKWw3Gu+q6MfDsFN76CAZ86tzSUbg==
=TE2o
-----END PGP SIGNATURE-----