Is this evaluation of conditions safe to write this way?

A

A

is it safer to write:

if (month < 1 || month > 12)
{
// month is safe here
if (day < 1 || day > monthDays[month-1]) // do something
}

rather than:

if (month < 1 || month > 12 || day < 1 || day > monthDays[month-1])
{
// do something
}

or maybe:

if ( (month < 1 || month > 12) || (day < 1 || day > monthDays[month-1]) )
{
// do something
}

in other words, is evaluation of parameters above something that is
dependent of the compiler or of the c++ standard? so that i don't get
undefined value in monthDays[month-1] if integer "month" is out of range? of
course, second and third version are easier to write but should it work
safely as far as monthDays[month-1] is considered?
 
J

Joshua Maurice

is it safer to write:

if (month < 1 || month > 12)
    {
    // month is safe here
    if (day < 1 || day > monthDays[month-1]) // do something
    }

I think you have a misunderstanding. Let me "translate" what you have
into English:
if (month < 1 || month > 12)
if month is less than 1, or if month is greater than 12,
{ then

// month is safe here
if (day < 1 || day > monthDays[month-1]) // do something
if days said:

I think you almost certainly want:
if (month >= 1&& month <= 12)
aka the English
if month is greater than or equal to one, and if month is less
than or equal to 12.

To answer your question though, there is no difference in effect
between any of the following (assuming no operator overloading), and I
don't see a large compelling difference offhand between any of the
following:

if (month >= 1 && month <= 12 && day >= 1 && day <=
monthDays[month-1])
//do something

if ( (month >= 1 && month <= 12) && (day >= 1 && day <=
monthDays[month-1]) )
//do something

if (month >= 1 && month <= 12)
{
if (day >= 1 && day <= monthDays[month-1])
{
//do something
}
}

The built-in operators || and && (when not overloaded) have what's
called "short-circuiting" behavior. That is, they will test the first
operand before evaluating the second operand. If the first operand of
&& is false, then the second operand will not be evaluated (and
equivalent for ||), making the following perfectly safe:
if (month >= 1 && month <= 12 && day >= 1 && day <=
monthDays[month-1])
In this case, you check that "month" is within the range of
"monthDays" before accessing "monthDays", so the short-circuiting
behavior will ensure that you don't get an out of bounds access.

Finally, dates are not trivial, and if you're using this in a large
project and not for learning, homework, nor fun, you should consider
looking for some open source library which does this already.
 
A

A

I think you almost certainly want:
if (month >= 1&& month <= 12)

I think you are almost certainly wrong :)

I want to check if month is OUT OF RANGE and not within range. If month is
within range, then next thing to check is if days are out of range. if
nothing is out of range, the statement is skipped. that is perfectly fine
for what i want.
To answer your question though, there is no difference in effect
between any of the following (assuming no operator overloading), and I
don't see a large compelling difference offhand between any of the
following:

I still believe you may be wrong here. If a compiler does left-to-right
evaluation then yes, there is no difference. But if there is no standard or
if compiler does not evaluate left to right it may happen that month is out
of range and the last check day > monthDays[month-1] fails.

So my question was does this depend on compiler or on some C++ standard.
From what I found the above could produce undefined behaviour so it is
recommended to put the correct order before doing evaluation not to depend
on left-to-right evaluation.
Finally, dates are not trivial, and if you're using this in a large
project and not for learning, homework, nor fun, you should consider
looking for some open source library which does this already.

what's not trivial about checking if month and days are within range? Leap
year is calculated by more complex function and the only difference is 28
vs. 29 days.
 
A

A

Finally, dates are not trivial, and if you're using this in a large
project and not for learning, homework, nor fun, you should consider
looking for some open source library which does this already.

Actually you gave me idea to look if C++ builder has a built-in function for
checking dates. I only found "IsLeapYear" before but now I see "IsValidDate"
is also there which more-less does the same thing as above. Thanks for that.

Still, my question about precedence of evaluation remains... I've
encountered that in other cases, not just this one.
 
A

Alf P. Steinbach /Usenet

* A, on 27.09.2010 21:26:
is it safer to write:

if (month< 1 || month> 12)
{
// month is safe here

Meaning (stated by you else-thread) that it's out of range. :)

if (day< 1 || day> monthDays[month-1]) // do something

Here you're indexing with a guaranteed out of range month.

Oops.

}

rather than:

if (month< 1 || month> 12 || day< 1 || day> monthDays[month-1])
{
// do something
}

It doesn't do the same, but this version is "safe", assuming that monthDays is
indexed from 0 to 11 inclusive.

or maybe:

if ( (month< 1 || month> 12) || (day< 1 || day> monthDays[month-1]) )
{
// do something
}

Still not the same as the first, but is the same as the second.

An expression equivalent to the first would be

if( (month < 1 || month > 12) && (day < 1 || day > someUpperLimit) )
{
// do something
}

Presumably that is not what you want.

in other words, is evaluation of parameters above something that is
dependent of the compiler or of the c++ standard? so that i don't get
undefined value in monthDays[month-1] if integer "month" is out of range?

The second and third versions are tecnically OK, but a bit obscure.

I'd do something like

bool validMonth( int m ) { return (1 <= m && m <= 12); }

bool validDay( int d, int m ) { return (1 <= d && d <= monthDays[m]); }


//...
if( !(validMonth( month ) && validDay( day, month )) )
{
// Do something.
}

And yes, C and C++ guarantee short-circuit evaluation of the *built-in* boolean
operators.

Pascal didn't/doesn't.

But if your lecturer says it does, don't react to that.

I did, once, in 1986. Put up my hand and said "nope".

Well, some discussion with the lecturer ensued, where he merely asserted
authority. He then disappeared for some minutes, returning with a book he'd
written, and quoted the book. It said the opposite of what he'd said earlier,
that is, his book was correct. I thought well then, that settles the matter. But
the guy put up this triumphant smile, and I forget what he said, but just about
everybody got the impression that I'd made an error, not him.

Argh.


Cheers & hth.,

- Alf

PS: If you're not familiar with it, look up de Morgans theorem/law. A nice
google exercise also, for free: in what week of what year did Augustus de Morgan
and George Boole both publish books, and what were the titles? Additional points
for finding the social I-know-you-know-someone-else thread from George Boole to
Charles Babbage (inventor of first programmable mechanical computer), goes like
Boole -> person X -> person Y -> Babbage. :)

PPS: Extra google challenge: what is the connection between beauty of small
women and ugliness of large ones, and de Morgan's infamous itchy poem? Hint:
it's not that the the small ones are fast.
 
P

Paul N

is it safer to write:

if (month < 1 || month > 12)
    {
    // month is safe here
    if (day < 1 || day > monthDays[month-1]) // do something
    }

This is wrong. You only get to the comment "month is safe here" if
month *isn't* safe. And this will make the final test go horribly
wrong.
rather than:

if (month < 1 || month > 12 || day < 1 || day > monthDays[month-1])
    {
    // do something
    }

This is fine, as far as I can tell. If you use ||, it will evaluate
the thing on the left. If it's non-zero, the result is true and it
won't go on to evaluate the thing on the right at all. If however the
thing on the left is zero, it will go on to evaluate the thing on the
right. This is guaranteed for all compilers. So here you only test
"day" if "month" is safe. If you get to "do something" then something
is wrong with either "month" or "day".
or maybe:

if ( (month < 1 || month > 12) || (day < 1 || day > monthDays[month-1]) )
    {
    // do something
    }

This will do exactly the same as the previous one.
in other words, is evaluation of parameters above something that is
dependent of the compiler or of the c++ standard? so that i don't get
undefined value in monthDays[month-1] if integer "month" is out of range? of
course, second and third version are easier to write but should it work
safely as far as monthDays[month-1] is considered?

No, || is one of the few operators where the evaluation order is
guaranteed. Others are && and the comma operator. (That's assuming you
haven't overloaded any of them, that is.) So your second and third
versions are safe.

Hope that helps.
Paul.
 
A

A

if (month said:
{
// month is safe here
if (day < 1 || day > monthDays[month-1]) // do something
}
This is wrong. You only get to the comment "month is safe here" if
month *isn't* safe. And this will make the final test go horribly
wrong.

yes you're right... i didn't look too much into this and made a mistake
there that's what caused this all mess...
i was translating from the original one line condition... so forgot to
change the condition above.

so to simplify this - if basically if the condition is:

if (a == 1 && b == 2 && c == 3)

then if a != 1 the result of entire condition will immediately evaluate to
"false" regardless of b and c values right? which would make for example
using of "a" variable more on the right in the condition safe if it is out
of range before?

and regardless of compiler?
 
A

A

Presumably that is not what you want.

yes yes... that's what i messed up but you got the point :)
I'd do something like
bool validMonth( int m ) { return (1 <= m && m <= 12); }
bool validDay( int d, int m ) { return (1 <= d && d <= monthDays[m]); }

cmon, having functions for this simple check, that's more obscure to me :)
And yes, C and C++ guarantee short-circuit evaluation of the *built-in*
boolean operators.
But if your lecturer says it does, don't react to that.

this was my lecturer :)
http://www.cppreference.com/wiki/operator_precedence
 
A

A

It's called "short-circuit evaluation", and it's guaranteed by the
standard.

OK, i googled a bit short-circuit evaluation and at least i know how it is
called now. but i did know about it before just didn't know if it is
guaranteed by the standard.
In this case I'd certainly prefer the third version, because it looks most
readable to me. Nevertheless, rather than reinventing the wheel and write
yet another custom Date class, have a look at the Boost date/time library.

well, c++ builder has already build in date/time functions that it shares
with delphi so no need for boost for that in this case.
 
A

A

Pascal didn't/doesn't.

well maybe true for 1986 pascal but modern pascals usually have compiler
directive that does this. it is {$B+} or {$B-} in delphi and probably
similar in free pascal.
 
R

robertwessel2

if (month < 1 || month > 12)
{
// month is safe here
if (day < 1 || day > monthDays[month-1]) // do something
}
This is wrong. You only get to the comment "month is safe here" if
month *isn't* safe. And this will make the final test go horribly
wrong.

yes you're right... i didn't look too much into this and made a mistake
there that's what caused this all mess...
i was translating from the original one line condition... so forgot to
change the condition above.

so to simplify this - if basically if the condition is:

if (a == 1 && b == 2 && c == 3)

then if a != 1 the result of entire condition will immediately evaluate to
"false" regardless of b and c values right? which would make for example
using of "a" variable more on the right in the condition safe if it is out
of range before?

and regardless of compiler?


So long as the || or && operator isn't user defined or overloaded, and
the compiler isn't horribly broken (a great deal of code depends on
short circuiting), then yes. With the caveat that the compiler can do
anything it wants so long as the result is the same from your
program's point of view. So for your example, if the compiler can
prove that accessing b and c is harmless (which would almost always be
the case for *this* example), it can, at its discretion, generate code
that does all three tests unconditionally - but that won't impact you
since the result *must* be as-if the expression evaluation had
actually been short-circuited.
 
K

Keith H Duggar

Presumably that is not what you want.

yes yes... that's what i messed up but you got the point :)
I'd do something like
  bool validMonth( int m ) { return (1 <= m && m <= 12); }
  bool validDay( int d, int m ) { return (1 <= d && d <= monthDays[m]); }

cmon, having functions for this simple check, that's more obscure to me :)

No it isn't more obscure it's /less/ obscure. It's less obscure
because it gives a clear semantic name to the check. Importantly
it also centralizes the check so that it can be modified easily
in one place rather than multiple places.

In fact, your original post post is an excellent example of why
such functions are good. Why? Because you screwed up the logic in
your first post:

if (month < 1 || month > 12)
{
// month is safe here
if (day < 1 || day > monthDays[month-1]) // failure
}

even though you got the comment right! Had you written your code
so it was self-commenting by providing semantically meaningful
names this mistake would have been far less likely. Example, it
would take quite some carelessness to write this:

if ( not validMonth(month) )
{
// month is safe here
if ( not validDay(day,month) ) // failure
}

where the comment is blatantly inconsistent from the now nearly
English code, rather than simply getting it right the first time
not even needing comments (since it is now self-documenting code):

if ( validMonth(month) and validDay(day,month) )
{
// do something valid
} else {
// failure
}

Embracing the concept of clear semantic naming even for things
that seem small will do wonders for your programming. Trust me.

KHD
 
J

Jorgen Grahn

OK, i googled a bit short-circuit evaluation and at least i know how it is
called now. but i did know about it before just didn't know if it is
guaranteed by the standard.

For some reason that's a pretty common misunderstanding, even though
it's a commonly used feature of both C and C++.

if(p && p[0]=='#') { ... }

if(foo.bad() || !do_something_to(foo)) complain();

Come to think of it, I cannot name one major language which
*doesn't* work like that. It just makes sense.

/Jorgen
 
G

gwowen

  if(p && p[0]=='#') { ... }
  if(foo.bad() || !do_something_to(foo)) complain();

Come to think of it, I cannot name one major language which
*doesn't* work like that.  It just makes sense.

Owen's third rule of programming languages: Whenever there's a
statement like "I cannot name one major language which *doesn't* work
like that" the counterexample is always Fortran.

e.g. in 'expr1.OR.expr2', if expr1 is logically true the compiler is
permitted to not evaluate expr2, but this is NOT required.
 
J

Jorgen Grahn

  if(p && p[0]=='#') { ... }
  if(foo.bad() || !do_something_to(foo)) complain();

Come to think of it, I cannot name one major language which
*doesn't* work like that.  It just makes sense.

Owen's third rule of programming languages: Whenever there's a
statement like "I cannot name one major language which *doesn't* work
like that" the counterexample is always Fortran.

e.g. in 'expr1.OR.expr2', if expr1 is logically true the compiler is
permitted to not evaluate expr2, but this is NOT required.

Ah, thanks. That makes sense too, Fortran being one of the oldest
languages.

What I still don't understand though is why the misconceptions are so
common. It's surely not because people are used to Fortran.

/Jorgen
 
J

James Kanze

For some reason that's a pretty common misunderstanding, even
though it's a commonly used feature of both C and C++.
if(p && p[0]=='#') { ... }
if(foo.bad() || !do_something_to(foo)) complain();
Come to think of it, I cannot name one major language which
*doesn't* work like that. It just makes sense.

Pascal required all of the expressions to be evaluated. And and
or in Ada do too (but Ada also offers "and then" and "or else"
with the short circuiting semantics).
 

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

No members online now.

Forum statistics

Threads
473,755
Messages
2,569,536
Members
45,014
Latest member
BiancaFix3

Latest Threads

Top