Can such style be justified?

A

Armen Tsirunyan

Hello,

if(true == someBool)
{
}

if(false == someBool)
{
}

bool someBool = (true == someBool1 || true == someBool2 && false ==
someBool3) ? false : true
{
}

One of my colleagues keeps writing such code. I didn't exaggerate a
bit. All three examples are taken from his code. He won't accept that
this is horrible style. But is this really a matter of taste or style?
I mean I want to puke every time I see such code. Am I biased? Can
such style be justified by any explanation, like making it more
readable (???!) or that prevents some errors?

Thank you for your opinions, I am going to show this thread to him :)
 
V

Vladimir Jovic

Armen said:
Hello,

if(true == someBool)
{
}

if(false == someBool)
{
}

I am using as above.
bool someBool = (true == someBool1 || true == someBool2 && false ==
someBool3) ? false : true
{
}

This is horrible. Why not like this?
const bool someBool = !( someBool1 || someBool2 && !someBool3 );
But I would remove the first negation from that expression (my brain
doesn't work at the moment, can't think how to do it right now).
 
V

Victor Bazarov

I am using as above.

Really?! So, why is the following "horrible"?
This is horrible. Why not like this?
const bool someBool = !( someBool1 || someBool2 && !someBool3 );
But I would remove the first negation from that expression (my brain
doesn't work at the moment, can't think how to do it right now).

Removing the negation requires changing all || to && and negating inner
expressions, from the top of my head.

V
 
G

gwowen

Hello,

if(true == someBool)

Style is i) subjective ii) of overrated importance. Some people like
this convention as

if(true = someBool) ...

then becomes a compilation error rather than a bug. I use
sufficiently strong compiler flags that I get warned anyway. Don't
care.
bool someBool = (true == someBool1 || true == someBool2 && false ==
someBool3) ? false : true
{

}

Style is overrated and subjective. Having said that, I don't like
this:
as its unclear to the casual reader where the precedence is.

Hmm, first I'd put some brackets in:
bool someBool = ((true == someBool1) || (true == someBool2 && false ==
someBool3)) ? false : true;

Quick - is that even right? Did I get C++'s precedence rules right?
That's why my style is to put brackets in to express intent.

Then I'd look to flip that horrible double negative...
bool someBool = not((true == someBool1) || (true == someBool2 && false
== someBool3));

then I'd do some Boolean algebra...
bool someBool = not(true == someBool1) && not(true == someBool2 &&
false == someBool3));

and some more
bool someBool = (true != someBool1) && (true != someBool2 || false !=
someBool3));
bool someBool = (false == someBool1) && (false == someBool2 || true ==
someBool3));

There, that looks better (to me).
 
G

gwowen

If someBool is what it's name suggests, there's no chance whatsoever of
making a mistake if you simply don't write the redundant comparison:

        if (someBool)

Good point.
 
V

Victor Bazarov

Too long and there is a hidden negation.

Why do you say it's hidden? Everything is pretty much spelled out. As
for the length, use a narrower font.

My point is, if you're so inclined to use verbose Boolean expressions,
then you can't call the last one "horrible", it's very inconsistent of you.

V
 
V

Vladimir Jovic

Victor said:
Why do you say it's hidden? Everything is pretty much spelled out. As
for the length, use a narrower font.

If the expression is true, the result is false, otherwise it is true.
That is hidden negation.
My point is, if you're so inclined to use verbose Boolean expressions,
then you can't call the last one "horrible", it's very inconsistent of you.

As gwowen did (elsethread) like this:
bool someBool = (false == someBool1) && (false == someBool2 || true ==
someBool3));
is much clearer then using the ?: operator.

btw you are right. Short boolean expressions are better :)
 
R

red floyd

Hello,

if(true == someBool)
{

}

if(false == someBool)
{

}

bool someBool = (true == someBool1 || true == someBool2 && false ==
someBool3) ? false : true
{

}

One of my colleagues keeps writing such code. I didn't exaggerate a
bit. All three examples are taken from his code. He won't accept that
this is horrible style. But is this really a matter of taste or style?
I mean I want to puke every time I see such code. Am I biased? Can
such style be justified by any explanation, like making it more
readable (???!) or that prevents some errors?

I've been at places where the coding standard document requires
exactly that.
I was not a happy camper.
 
P

Puppet_Sock

Hello,

if(true == someBool)
{

}

if(false == someBool)
{

}

bool someBool = (true == someBool1 || true == someBool2 && false ==
someBool3) ? false : true
{

}

One of my colleagues keeps writing such code. I didn't exaggerate a
bit. All three examples are taken from his code. He won't accept that
this is horrible style. But is this really a matter of taste or style?
I mean I want to puke every time I see such code. Am I biased? Can
such style be justified by any explanation, like making it more
readable (???!) or that prevents some errors?

You don't actually say what you think the bad style is here.
Downthread people concluded it was the explicit compare to
true or false. Others concluded it was the long formula and
the seemingly unrequired complexity. Nobody thought it was
the empty brackets in your examples. Hmmm...

In the examples you give there seems no reason to do the
compares. However, suppose that instead of comparing to
true and false, instead the comparison was to operating
and not_operating. Or some other symbol that was a bool
constant that indicated some binary state value of some
thing the code was modelling. Example:

if(isNotLit ==
operatorPanel.GetLightState(OpPanel_WarningLights_FullTank))
{
// do some stuff
}

Now there is clearly some value to the style. The value true,
the name true, does not communicate what is going on with
the warning light. With isLit in there, you have built
in documentation of what the code is trying to do.

And more, it now hides the detail of the specific type of
variable that is being used. Suppose that the light gets
a new state later on. Maybe not just lit or unlit, but
blinking. So when you get asked to upgrade the system,
say so the light is off with an empty tank, green for low,
orange for mid, red for high, and blinking for a change,
then it's very easy with the compare in there.
Just change the value to an integer, or maybe an enum,
change the constant isNotLit to the appropriate value,
expand the set of values from isLit and isNotLit, and
go on.
Socks
 
J

Jim Langston

Hello,

if(true == someBool)
{

}

You could do:
if ( someBool )
{
}

here. Is that "better style" or not so "horrible style"? In C it was/
is common to not compare booleans to true or false. Also in C however
it is very common to treat any number as a boolean.

if ( quantity )
{
}

which is almost the same thing, but I personally think is bad style
and would do this:

if ( quantity != 0 )
{
}

Now, if I see any of this code, I know what it does. Personally, I
don't compare booleans to true or false, but have done so in the past
to make some things more clear.

[snip]
bool someBool = (true == someBool1 || true == someBool2 && false ==
someBool3) ? false : true
{

}

This is just logic. Looks to be something like
if (( someBool1 || someBool2 ) && ! someBool3 )
{
}

(or is that if ( soemBool1 || ( someBool2 || !someBool3 ) )

I would on that last one use an if statement and spell it out just so
it's easier to read. But bad style? Meh, I've seen worst. I might
tell a programmer they do no have to compare to boolean and they
should use more parenthesis to make the order of operations clearer
but I could live with this code.
 
F

Francesco S. Carta

if(true == someBool)
{
}

if(false == someBool)
{
}

I prefer the "if(somebool)" style, just like Pete.
bool someBool = (true == someBool1 || true == someBool2&& false ==
someBool3) ? false : true

Don't know how logical this is going to seem, when looked from a
different point of view, but this is one of the cases where I'd prefer
to be verbose, although in a different way, and to assign more
importance to the test:

bool someBool = true;
if(someBool1 || someBool2 && !someBool3) {
someBool = false;
}

In any case, the example at hand is too generic, such stuff should be
pondered in a real case: depending on the surrounding code (and on the
real names of those bools) I could synthesize it back or even "unroll"
it further, if I can take advantage of it in order to do other stuff in
the mean time.
 
A

Alf P. Steinbach /Usenet

* Armen Tsirunyan, on 23.09.2010 15:58:
if(true == someBool)
{
}

if(false == someBool)
{
}

bool someBool = (true == someBool1 || true == someBool2&& false ==
someBool3) ? false : true
{
}

One of my colleagues keeps writing such code. I didn't exaggerate a
bit. All three examples are taken from his code. He won't accept that
this is horrible style. But is this really a matter of taste or style?

No, it's not a matter of style.

It would ordinarily mean that your colleague is not just currently incompetent,
but too stupid to learn.

Alternatively he or she might be doing it for unsavory motives such as job
security through code obscurity, or just to provoke you and/or others.

I mean I want to puke every time I see such code. Am I biased? Can
such style be justified by any explanation, like making it more
readable (???!) or that prevents some errors?

Such a style cannot be justified.

Thank you for your opinions, I am going to show this thread to him :)

He he. :)


Cheers & hth.,

- Alf
 
I

Ian Collins

Hello,

if(true == someBool)
{
}

if(false == someBool)
{
}

Awful.

I assume to be consistent your colleague also writes

if(true == (42 == n))

for integer comparisons?
 
J

Joshua Maurice

Hello,

if(true == someBool)
{

}

if(false == someBool)
{

}

bool someBool = (true == someBool1 || true == someBool2 && false ==
someBool3) ? false : true
{

}

One of my colleagues keeps writing such code. I didn't exaggerate a
bit. All three examples are taken from his code. He won't accept that
this is horrible style. But is this really a matter of taste or style?
I mean I want to puke every time I see such code. Am I biased? Can
such style be justified by any explanation, like making it more
readable (???!) or that prevents some errors?

Thank you for your opinions, I am going to show this thread to him :)

IMO, this is pretty bad.

I would argue that, in general, there will be some styles which help
the reader better understand the code and the writer better write the
code as compared to other styles. However, I highly doubt most people
are quoting scientifically rigorous studies when saying which style is
better for things such as "where to put your curly braces", and to a
lesser extent variable naming conventions (like m_memberVariables). I
also suspect that these small style differences aren't terribly
significant to programmer productivity.

However, there are some more clear cut cases, like the above. I admit
that I like hard scientific studies on it, but it would be my strong
supposition that, all other things being equal, writing
if (true == someBoolean)
over
if (someBoolean)
is stupid. It adds no value - it is not clearer, does not help prevent
mistakes, does not increase runtime efficiency, and it definitely
subtracts value - it takes me longer to read.

When one is writing code professional, one should keep it as short as
possible while keeping it understandable, maintainable, efficient,
etc., but no shorter. It is quite unprofessional to add verbosity for
no reason.

(I admit the small possibility that it makes sense and fits with a
larger style. If so, deviating from that overarching would make code
harder to read, so in isolation it really is hard quite to
definitively say it's bad. However, I would likely also label that
overarching style as atrocious.)
 
G

gwowen

I prefer:
   bool someBool = !someBool1 && (!someBool2 || someBool3);

It is more concise and should be immediately understandable to any
professional C or C++ programmer (as well as programmers of many other
languages.)

Sure. That's probably what I'd do. I was restricting my transformation
to the logic, and retaining the original style. I don't care for the
original style, but the ?: construction was far more obfuscating that
the (false == var) stylistic quirk.
 
G

gwowen

Call me a hard ass, but I think the understanding of precedence is a
basic skill that every programmer should know.

I understand precedence. I just don't rely I on language rules when
its just as easy to code unambiguously. I can't be bothered to
memorise the precise precedence rules of every language in which I
program -- and no, they are not all the same. Writing code that
expresses intent with brackets rather than relying on my (flawed)
memory means I make fewer mistakes, and makes it easier for beginner
programmers to grok the code as the read it. Making life easier for
maintainers makes my life easier.
 
G

Goran

Another possibility is to put this into a function with a meaningful name:

inline bool should_do_it()
        {
        return someBool1 || someBool2 && !someBool3;
        }

Big +1 for this over here. Code is made to be read, first and
foremost. A function like this eliminates discrepancy between lower-
level conditional munching and higher-level overall program logic.

if (should_do_it(bool1, bool2, bool3)
{
}

reads like a charm with an appropriate name for should_do_it.

Chances are, there is no perf impact in this, and even if there was,
it would not be really measurable. Downside here is increased "ravioli-
ness", but that's IMO a small price to pay for better clarity.

Goran.
 
S

Stuart Redmann

Hello,

if(true == someBool)
{

}

if(false == someBool)
{

}

bool someBool = (true == someBool1 || true == someBool2 && false ==
someBool3) ? false : true
{

}

One of my colleagues keeps writing such code. I didn't exaggerate a
bit. All three examples are taken from his code. He won't accept that
this is horrible style. But is this really a matter of taste or style?
I mean I want to puke every time I see such code. Am I biased? Can
such style be justified by any explanation, like making it more
readable (???!) or that prevents some errors?

Thank you for your opinions, I am going to show this thread to him :)

Comparison in conjunction with bool is considered EVIL (tm) by several
people, see http://blogs.msdn.com/b/oldnewthing/archive/2004/12/22/329884.aspx.
Although C++ makes it a bit difficult to misuse bool, we can write
applications that show some of the problems (note that the following
program formally shows UB, but you get the idea):

#include <iostream>

union BoolInt
{
int i;
bool b;
};

int main ()
{
BoolInt bi;
bi.i = 4;
bool b = bi.b;

if (true == b)
std::cout << "(true == b) is true" << std::endl;
else
std::cout << "(true == b) is false" << std::endl;

if (b)
std::cout << "(b) is true" << std::endl;
else
std::cout << "(b) is false" << std::endl;
}

Comparisons with other boolean data types like BOOL, VARIANT_BOOL and
so on will also lead to problems. Some would argue that BOOL and
VARIANT_BOOL are evil themselves, but some of us still have to work
with them every day.

I think it would be the best if there were no comparison operator for
bool (I'd rather have a build-in XOR operator for bool).

Regards,
Stuart
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
474,145
Messages
2,570,826
Members
47,371
Latest member
Brkaa

Latest Threads

Top