I read some legacy code:
if ( 1 || !Foo() )
Is there any seen reason why not to write:
if ( !Foo() )
Join Stack Overflow to learn, share knowledge, and build your career.
I read some legacy code:
if ( 1 || !Foo() )
Is there any seen reason why not to write:
if ( !Foo() )
The two are not the same. The first will never evaluate Foo()
because the 1
short-circuits the ||
.
Why it's done - probably someone wanted to force entry in the then
branch for debugging purposes and left it there. It could also be that this was written before source control, so they didn't want the code to be lost, rather just bypassed for now.
if (1 || !Foo() )
will be always satisfied. !Foo()
will not even be reached because of short-circuits evaluation.
This happens when you want to make sure that the code below the if
will be executed, but you don't want to remove the real condition in it, probably for debug purposes.
Additional information that might help you:
if(a && b)
- if a
is false
, b
won't be checked. if(a && b)
- if a
is true
, b
will be checked, because if it's false
, the expression will be false
.if(a || b)
- if a
is true
, b
won't be checked, because this is true
anyway.if(a || b)
- if a
is false
, b
will be checked, because if b
is true
then it'll be true
.It's highly recommended to have a macro for this purpose, say DEBUG_ON 1
, that will make it easier to understand what the programmer means, and not to have magic numbers in the code (Thanks @grigeshchauhan).
if(1 || !Foo() ) {code}
== if(1){code}
== {code}
while if (!Foo())
can be either {code}
or just ;
depends on value returned by Foo()
. :)
– Grijesh Chauhan
Oct 10 '13 at 9:56
DEGUG_ON = 1
or comment it. if (DEGUG_ON || !Foo())
(in fact I will comment code). ;)
– Grijesh Chauhan
Oct 10 '13 at 14:52
1 || condition
is always true, regardless whether the condition
is true or not. In this case, the condition
is never even being evaluated. The following code:
int c = 5;
if (1 || c++){}
printf("%d", c);
outputs 5
since c
is never incremented, however if you changed 1
to 0
, the c++
would be actually called, making the output 6
.
A usual practical usage of this is in the situation when you want to test some piece of code that is being invoked when the condition that evaluates to true only seldom is met:
if (1 || condition ) {
// code I want to test
}
This way condition
will never be evaluated and therefore // code I want to test
always invoked. However it is definitely not the same as:
if (condition) { ...
which is a statement where condition
will actually be evaluated (and in your case Foo
will be called)
you don't want to wait till that condition will evaluate to true
yes basically short-circuits is a technique to save executions!
– Grijesh Chauhan
Oct 10 '13 at 10:06
The question was answered properly - the difference is the right side of the or operation is short-circuited, suggesting this is debug code to force entry into the if block.
But in the interest of best practices, at least my rough stab at a best practice, I'd suggest alternatives, in order of increasing preference (best is last):
note: noticed after I coded examples this was a C++ question, examples are C#. Hopefully you can translate. If anyone needs me to, just post a comment.
In-line comment:
if (1 /*condition*/) //temporary debug
Out-of-line comment:
//if(condition)
if(true) //temporary debug
Name-Indicative Function
//in some general-use container
bool ForceConditionForDebug(bool forcedResult, string IgnoredResult)
{
#if DEBUG
Debug.WriteLine(
string.Format(
"Conditional {0} forced to {1} for debug purposes",
IgnoredResult,
forcedResult));
return forcedResult;
#else
#if ALLOW_DEBUG_CODE_IN_RELEASE
return forcedResult;
#else
throw new ApplicationException("Debug code detected in release mode");
#endif
#endif
}
//Where used
if(ForceConditionForDebug(true, "condition"))...
//Our case
if(ForceConditionForDebug(true, "!Foo()"))...
And if you wanted a really robust solution, you could add a repository rule to source control to reject any checked in code that called ForceConditionForDebug. This code should never have been written that way because it obviously doesn't communicate intent. It never should have been checked in (or have been allowed to be checked in) (source control? peer review?) And it should definitely never be allowed to execute in production in its current form.
if (1 || !foo())
<==>if (1)
. – devnull Oct 10 '13 at 9:45return rand() % 2;
– Happy Green Kid Naps Oct 10 '13 at 16:271 && !Foo()
is equal to!Foo()
– Niklas R Oct 12 '13 at 9:10