If you want to some code to execute based on two or more conditions which is the best way to format that if statement ?
first example:-
if(ConditionOne && ConditionTwo && ConditionThree)
{
Code to execute
}
Second example:-
if(ConditionOne)
{
if(ConditionTwo )
{
if(ConditionThree)
{
Code to execute
}
}
}
which is easiest to understand and read bearing in mind that each condition may be a long function name or something.
This question is related to
if-statement
formatting
language-agnostic
I prefer Option A
bool a, b, c;
if( a && b && c )
{
//This is neat & readable
}
If you do have particularly long variables/method conditions you can just line break them
if( VeryLongConditionMethod(a) &&
VeryLongConditionMethod(b) &&
VeryLongConditionMethod(c))
{
//This is still readable
}
If they're even more complicated, then I'd consider doing the condition methods separately outside the if statement
bool aa = FirstVeryLongConditionMethod(a) && SecondVeryLongConditionMethod(a);
bool bb = FirstVeryLongConditionMethod(b) && SecondVeryLongConditionMethod(b);
bool cc = FirstVeryLongConditionMethod(c) && SecondVeryLongConditionMethod(c);
if( aa && bb && cc)
{
//This is again neat & readable
//although you probably need to sanity check your method names ;)
}
IMHO The only reason for option 'B' would be if you have separate else functions to run for each condition.
e.g.
if( a )
{
if( b )
{
}
else
{
//Do Something Else B
}
}
else
{
//Do Something Else A
}
The first example is more "easy to read".
Actually, in my opinion you should only use the second one whenever you have to add some "else logic", but for a simple Conditional, use the first flavor. If you are worried about the long of the condition you always can use the next syntax:
if(ConditionOneThatIsTooLongAndProbablyWillUseAlmostOneLine
&& ConditionTwoThatIsLongAsWell
&& ConditionThreeThatAlsoIsLong) {
//Code to execute
}
Good Luck!
Other answers explain why the first option is normally the best. But if you have multiple conditions, consider creating a separate function (or property) doing the condition checks in option 1. This makes the code much easier to read, at least when you use good method names.
if(MyChecksAreOk()) { Code to execute }
...
private bool MyChecksAreOk()
{
return ConditionOne && ConditionTwo && ConditionThree;
}
It the conditions only rely on local scope variables, you could make the new function static and pass in everything you need. If there is a mix, pass in the local stuff.
The first example is more "easy to read".
Actually, in my opinion you should only use the second one whenever you have to add some "else logic", but for a simple Conditional, use the first flavor. If you are worried about the long of the condition you always can use the next syntax:
if(ConditionOneThatIsTooLongAndProbablyWillUseAlmostOneLine
&& ConditionTwoThatIsLongAsWell
&& ConditionThreeThatAlsoIsLong) {
//Code to execute
}
Good Luck!
The question was asked and has, so far, been answered as though the decision should be made purely on "syntactic" grounds.
I would say that the right answer of how you lay-out a number of conditions within an if, ought to depend on "semantics" too. So conditions should be broken up and grouped according to what things go together "conceptually".
If two tests are really two sides of the same coin eg. if (x>0) && (x<=100) then put them together on the same line. If another condition is conceptually far more distant eg. user.hasPermission(Admin()) then put it on it's own line
Eg.
if user.hasPermission(Admin()) {
if (x >= 0) && (x < 100) {
// do something
}
}
When condition is really complex I use the following style (PHP real life example):
if( $format_bool &&
(
( isset( $column_info['native_type'] )
&& stripos( $column_info['native_type'], 'bool' ) !== false
)
|| ( isset( $column_info['driver:decl_type'] )
&& stripos( $column_info['driver:decl_type'], 'bool' ) !== false
)
|| ( isset( $column_info['pdo_type'] )
&& $column_info['pdo_type'] == PDO::PARAM_BOOL
)
)
)
I believe it's more nice and readable than nesting multiple levels of if(). And in some cases like this you simply can't break complex condition into pieces because otherwise you would have to repeat the same statements in if() {...} block many times.
I also believe that adding some "air" into code is always a good idea. It improves readability greatly.
The second one is a classic example of the Arrow Anti-pattern So I'd avoid it...
If your conditions are too long extract them into methods/properties.
{
( VeryLongCondition_1 ) or last;
( VeryLongCondition_2 ) or last;
( VeryLongCondition_3 ) or last;
( VeryLongCondition_4 ) or last;
( VeryLongCondition_5 ) or last;
( VeryLongCondition_6 ) or last;
# Guarded code goes here
}
If any of the conditions fail it will just continue on, after the block. If you are defining any variables that you want to keep around after the block, you will need to define them before the block.
I've been facing this dilemma for a long time and I still can't find a proper solution. In my opinion only good way is to first try to get rid of conditions before so you're not suddenly comparing 5 of them.
If there's no alternative then like others have suggested - break it down into separete ones and shorten the names or group them and e.g. if all must be true then use something like "if no false in array of x then run".
If all fails @Eoin Campbell gave pretty good ideas.
Other answers explain why the first option is normally the best. But if you have multiple conditions, consider creating a separate function (or property) doing the condition checks in option 1. This makes the code much easier to read, at least when you use good method names.
if(MyChecksAreOk()) { Code to execute }
...
private bool MyChecksAreOk()
{
return ConditionOne && ConditionTwo && ConditionThree;
}
It the conditions only rely on local scope variables, you could make the new function static and pass in everything you need. If there is a mix, pass in the local stuff.
The second one is a classic example of the Arrow Anti-pattern So I'd avoid it...
If your conditions are too long extract them into methods/properties.
The question was asked and has, so far, been answered as though the decision should be made purely on "syntactic" grounds.
I would say that the right answer of how you lay-out a number of conditions within an if, ought to depend on "semantics" too. So conditions should be broken up and grouped according to what things go together "conceptually".
If two tests are really two sides of the same coin eg. if (x>0) && (x<=100) then put them together on the same line. If another condition is conceptually far more distant eg. user.hasPermission(Admin()) then put it on it's own line
Eg.
if user.hasPermission(Admin()) {
if (x >= 0) && (x < 100) {
// do something
}
}
Other answers explain why the first option is normally the best. But if you have multiple conditions, consider creating a separate function (or property) doing the condition checks in option 1. This makes the code much easier to read, at least when you use good method names.
if(MyChecksAreOk()) { Code to execute }
...
private bool MyChecksAreOk()
{
return ConditionOne && ConditionTwo && ConditionThree;
}
It the conditions only rely on local scope variables, you could make the new function static and pass in everything you need. If there is a mix, pass in the local stuff.
if ( ( single conditional expression A )
&& ( single conditional expression B )
&& ( single conditional expression C )
)
{
opAllABC();
}
else
{
opNoneABC();
}
Formatting a multiple conditional expressions in an if-else statement this way:
// disable any single conditional test with just a pre-pended '//'
// set a break point before any individual test
// syntax '(1 &&' and '(0 ||' usually never creates any real code
if ( 1
&& ( single conditional expression A )
&& ( single conditional expression B )
&& ( 0
|| ( single conditional expression C )
|| ( single conditional expression D )
)
)
{
... ;
}
else
{
... ;
}
The second one is a classic example of the Arrow Anti-pattern So I'd avoid it...
If your conditions are too long extract them into methods/properties.
When condition is really complex I use the following style (PHP real life example):
if( $format_bool &&
(
( isset( $column_info['native_type'] )
&& stripos( $column_info['native_type'], 'bool' ) !== false
)
|| ( isset( $column_info['driver:decl_type'] )
&& stripos( $column_info['driver:decl_type'], 'bool' ) !== false
)
|| ( isset( $column_info['pdo_type'] )
&& $column_info['pdo_type'] == PDO::PARAM_BOOL
)
)
)
I believe it's more nice and readable than nesting multiple levels of if(). And in some cases like this you simply can't break complex condition into pieces because otherwise you would have to repeat the same statements in if() {...} block many times.
I also believe that adding some "air" into code is always a good idea. It improves readability greatly.
{
( VeryLongCondition_1 ) or last;
( VeryLongCondition_2 ) or last;
( VeryLongCondition_3 ) or last;
( VeryLongCondition_4 ) or last;
( VeryLongCondition_5 ) or last;
( VeryLongCondition_6 ) or last;
# Guarded code goes here
}
If any of the conditions fail it will just continue on, after the block. If you are defining any variables that you want to keep around after the block, you will need to define them before the block.
I prefer Option A
bool a, b, c;
if( a && b && c )
{
//This is neat & readable
}
If you do have particularly long variables/method conditions you can just line break them
if( VeryLongConditionMethod(a) &&
VeryLongConditionMethod(b) &&
VeryLongConditionMethod(c))
{
//This is still readable
}
If they're even more complicated, then I'd consider doing the condition methods separately outside the if statement
bool aa = FirstVeryLongConditionMethod(a) && SecondVeryLongConditionMethod(a);
bool bb = FirstVeryLongConditionMethod(b) && SecondVeryLongConditionMethod(b);
bool cc = FirstVeryLongConditionMethod(c) && SecondVeryLongConditionMethod(c);
if( aa && bb && cc)
{
//This is again neat & readable
//although you probably need to sanity check your method names ;)
}
IMHO The only reason for option 'B' would be if you have separate else functions to run for each condition.
e.g.
if( a )
{
if( b )
{
}
else
{
//Do Something Else B
}
}
else
{
//Do Something Else A
}
The first one is easier, because, if you read it left to right you get: "If something AND somethingelse AND somethingelse THEN" , which is an easy to understand sentence. The second example reads "If something THEN if somethingelse THEN if something else THEN", which is clumsy.
Also, consider if you wanted to use some ORs in your clause - how would you do that in the second style?
Other answers explain why the first option is normally the best. But if you have multiple conditions, consider creating a separate function (or property) doing the condition checks in option 1. This makes the code much easier to read, at least when you use good method names.
if(MyChecksAreOk()) { Code to execute }
...
private bool MyChecksAreOk()
{
return ConditionOne && ConditionTwo && ConditionThree;
}
It the conditions only rely on local scope variables, you could make the new function static and pass in everything you need. If there is a mix, pass in the local stuff.
I prefer Option A
bool a, b, c;
if( a && b && c )
{
//This is neat & readable
}
If you do have particularly long variables/method conditions you can just line break them
if( VeryLongConditionMethod(a) &&
VeryLongConditionMethod(b) &&
VeryLongConditionMethod(c))
{
//This is still readable
}
If they're even more complicated, then I'd consider doing the condition methods separately outside the if statement
bool aa = FirstVeryLongConditionMethod(a) && SecondVeryLongConditionMethod(a);
bool bb = FirstVeryLongConditionMethod(b) && SecondVeryLongConditionMethod(b);
bool cc = FirstVeryLongConditionMethod(c) && SecondVeryLongConditionMethod(c);
if( aa && bb && cc)
{
//This is again neat & readable
//although you probably need to sanity check your method names ;)
}
IMHO The only reason for option 'B' would be if you have separate else functions to run for each condition.
e.g.
if( a )
{
if( b )
{
}
else
{
//Do Something Else B
}
}
else
{
//Do Something Else A
}
if ( ( single conditional expression A )
&& ( single conditional expression B )
&& ( single conditional expression C )
)
{
opAllABC();
}
else
{
opNoneABC();
}
Formatting a multiple conditional expressions in an if-else statement this way:
// disable any single conditional test with just a pre-pended '//'
// set a break point before any individual test
// syntax '(1 &&' and '(0 ||' usually never creates any real code
if ( 1
&& ( single conditional expression A )
&& ( single conditional expression B )
&& ( 0
|| ( single conditional expression C )
|| ( single conditional expression D )
)
)
{
... ;
}
else
{
... ;
}
I prefer Option A
bool a, b, c;
if( a && b && c )
{
//This is neat & readable
}
If you do have particularly long variables/method conditions you can just line break them
if( VeryLongConditionMethod(a) &&
VeryLongConditionMethod(b) &&
VeryLongConditionMethod(c))
{
//This is still readable
}
If they're even more complicated, then I'd consider doing the condition methods separately outside the if statement
bool aa = FirstVeryLongConditionMethod(a) && SecondVeryLongConditionMethod(a);
bool bb = FirstVeryLongConditionMethod(b) && SecondVeryLongConditionMethod(b);
bool cc = FirstVeryLongConditionMethod(c) && SecondVeryLongConditionMethod(c);
if( aa && bb && cc)
{
//This is again neat & readable
//although you probably need to sanity check your method names ;)
}
IMHO The only reason for option 'B' would be if you have separate else functions to run for each condition.
e.g.
if( a )
{
if( b )
{
}
else
{
//Do Something Else B
}
}
else
{
//Do Something Else A
}
The question was asked and has, so far, been answered as though the decision should be made purely on "syntactic" grounds.
I would say that the right answer of how you lay-out a number of conditions within an if, ought to depend on "semantics" too. So conditions should be broken up and grouped according to what things go together "conceptually".
If two tests are really two sides of the same coin eg. if (x>0) && (x<=100) then put them together on the same line. If another condition is conceptually far more distant eg. user.hasPermission(Admin()) then put it on it's own line
Eg.
if user.hasPermission(Admin()) {
if (x >= 0) && (x < 100) {
// do something
}
}
The first one is easier, because, if you read it left to right you get: "If something AND somethingelse AND somethingelse THEN" , which is an easy to understand sentence. The second example reads "If something THEN if somethingelse THEN if something else THEN", which is clumsy.
Also, consider if you wanted to use some ORs in your clause - how would you do that in the second style?
The first example is more "easy to read".
Actually, in my opinion you should only use the second one whenever you have to add some "else logic", but for a simple Conditional, use the first flavor. If you are worried about the long of the condition you always can use the next syntax:
if(ConditionOneThatIsTooLongAndProbablyWillUseAlmostOneLine
&& ConditionTwoThatIsLongAsWell
&& ConditionThreeThatAlsoIsLong) {
//Code to execute
}
Good Luck!
{
( VeryLongCondition_1 ) or last;
( VeryLongCondition_2 ) or last;
( VeryLongCondition_3 ) or last;
( VeryLongCondition_4 ) or last;
( VeryLongCondition_5 ) or last;
( VeryLongCondition_6 ) or last;
# Guarded code goes here
}
If any of the conditions fail it will just continue on, after the block. If you are defining any variables that you want to keep around after the block, you will need to define them before the block.
The first one is easier, because, if you read it left to right you get: "If something AND somethingelse AND somethingelse THEN" , which is an easy to understand sentence. The second example reads "If something THEN if somethingelse THEN if something else THEN", which is clumsy.
Also, consider if you wanted to use some ORs in your clause - how would you do that in the second style?
The second one is a classic example of the Arrow Anti-pattern So I'd avoid it...
If your conditions are too long extract them into methods/properties.
The first one is easier, because, if you read it left to right you get: "If something AND somethingelse AND somethingelse THEN" , which is an easy to understand sentence. The second example reads "If something THEN if somethingelse THEN if something else THEN", which is clumsy.
Also, consider if you wanted to use some ORs in your clause - how would you do that in the second style?
{
( VeryLongCondition_1 ) or last;
( VeryLongCondition_2 ) or last;
( VeryLongCondition_3 ) or last;
( VeryLongCondition_4 ) or last;
( VeryLongCondition_5 ) or last;
( VeryLongCondition_6 ) or last;
# Guarded code goes here
}
If any of the conditions fail it will just continue on, after the block. If you are defining any variables that you want to keep around after the block, you will need to define them before the block.
The question was asked and has, so far, been answered as though the decision should be made purely on "syntactic" grounds.
I would say that the right answer of how you lay-out a number of conditions within an if, ought to depend on "semantics" too. So conditions should be broken up and grouped according to what things go together "conceptually".
If two tests are really two sides of the same coin eg. if (x>0) && (x<=100) then put them together on the same line. If another condition is conceptually far more distant eg. user.hasPermission(Admin()) then put it on it's own line
Eg.
if user.hasPermission(Admin()) {
if (x >= 0) && (x < 100) {
// do something
}
}
The first example is more "easy to read".
Actually, in my opinion you should only use the second one whenever you have to add some "else logic", but for a simple Conditional, use the first flavor. If you are worried about the long of the condition you always can use the next syntax:
if(ConditionOneThatIsTooLongAndProbablyWillUseAlmostOneLine
&& ConditionTwoThatIsLongAsWell
&& ConditionThreeThatAlsoIsLong) {
//Code to execute
}
Good Luck!
Source: Stackoverflow.com