-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 On 01/13/2015 08:25 AM, Timothy Pearson wrote: >> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 > >> <big snip> > >>>> 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? > >> Yes, but an operator is more visible than a missing brace :-) > >>> >>>> 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. >>> > >> 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. ;-) No problem, not a big deal. > >>>> 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? > >> 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? Up to you and Slavek, both ways are ok for me. >> 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. > ok, I see the point. Cheers Michele -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBCgAGBQJUtFo5AAoJECp1t8qK3tXPMVcP/1KL33VWgHreUbeJXMcylmSY NzOn+mqITsZ+U+Mr1HWoX7KTEfgK0JsnEED5xJqfb+MsBNB+db+H0ky2BZTFTVve kkCKtzrVcZ11H0oR6UGLjzhXeYgv2jIgara3UhmhRfnYKoNZHShXPidFLFXWOaGS pqjQsp/ZUkr7khnVenrli/PlpzbQtj6SdM+4ncx1sWBxQKttRdXHJUPxNZm9MBzj 3yqaBogj3hBlD0xWBUx82NlbDLMbtquwYUYDjUUCyCbNvylVdD9f6Ie9Xn72lSIV +pqAkKwS7dmfrFjNFgLQ8W86kSDy5/n7h6J/S7xrqYP7R/bgSPPP3tma3hsda0gl xsbPxsR29A0yQ2gno1OZ0xjjWQqxM+0pGABm9NsbYOWa/dT8VJ1jPt4Khi5305nz 00rKeBKDxbP+xfP4UVf823kFLkZQdAsBeA6sCe6A+9wK6yGLXijtcMnXg6fQzdJg qSwYlAHk0MAXhWYXJcSNHFGttAPVjWydmzOQsAvE41AU6L+FW7Hd2sbNo83GxLQy Jf0WhDGSixeUNj5ZMiOO8hWW1g9qCzV9aHde26Nbzxc6qt/Z3gPOIMqQjRT6VD9t DC5vNhYrbinsWX8EUoSbd3WmuFZphzULV0FrsPZ6GjTSpaaDbMqikTM916Pfb7dk rV2w6EJ+WU2ukpQPCKSH =8Ljh -----END PGP SIGNATURE-----