Which is the better method to use "if else" statements
in [Embedded]
|
From: Spehro Pefhany on 7 Jun 2010 10:26 On Mon, 7 Jun 2010 06:27:37 -0700 (PDT), rickman <gnuarm(a)gmail.com> wrote: >There is one other way to write this that I tend to use when coding. >One of the "guides" to coding I read many years ago, perhaps in >college said the smaller of the two choices in an if then else should >be first. This makes it easier for the eye to scan the code and glean >structure from the shape... a bit like the way I learned to read... >look for the outline of the words rather than parsing every letter. > >Also,, it is not uncommon for the else and the if to be put on the >same line to show that the "if" is the only statement within the >else. The result is very clean, simple and uses much less space >streamlining the code to the point that the structure is very clear to >the eye. Oh, and you don't need all those parens for single >statements. > >uint16_t Func1(uint16_t int1, uint16_t int2, uint16_t int3) >{ > if (int1 != 32) > return 3; > else if (int2 != 16) > return 2; > else if (int3 != 8) > return 1; > else > return 0; >} > >Rick Why waste all those linefeeds when you can simply write uint16_t func(uint16_t i1, uint16_t i2, uint16_t i3) {return i1 != 32 ? 3: i2 != 16 ? 2: i3 !=8 ? 1 : 0;} ? ;-)
From: Phil on 7 Jun 2010 13:59 On Jun 7, 8:48 am, Vladimir Vassilevsky <nos...(a)nowhere.com> wrote: > kishor wrote: > > Hi friends, > > > I have some doubt in using "if" "else" statements. > > Which is the better method (for efficiency, readability or any other > > reason)????. > > A piece of code shall have only one entry point and only one exit point. This is the sort of discussion in which reasonable people can reasonably disagree. I know that is the conventional structured programming wisdom, but I have seen far too many situations in which obeying that dictum results in code getting a lot harder to understand. The point of that rule (which I follow as often as I can) is to make sure you don't hack an unstructured GOTO via a return statement stuck in the middle of a module. My own guideline is that there should be only a single exit point OR exit points should be in a highly regular pattern (e.g., a return inside every if statement). In other words, regular structure is what matters to help in understanding the code. If you want to obey the single-exit rule, you can usually use a chain of "else if" clauses. Depends upon your taste and training. The downside is you often need to keep a variable around to store the return value. The other responses in this thread have put forth many good ideas. Which ideas are best depends on the context. But I will add one tradeoff rule to the mix: Minimize the number of levels of if statement nesting to the extent practical. If you have more than 3 or 4 levels of nesting, you're asking for bugs. A chain of "else if" clauses probably doesn't count as nesting so long as there is no plain "else" the chain (in effect, an if-else chain without an intervening else is a more generalized form of a switch statement). Phil Koopman -- koopman(a)cmu.edu -- http://www.ece.cmu.edu/~koopman Author of: Better Embedded System Software, Drumnadrochit Press 2010
From: -jg on 7 Jun 2010 15:17 On Jun 8, 1:27 am, rickman <gnu...(a)gmail.com> wrote: > uint16_t Func1(uint16_t int1, uint16_t int2, uint16_t int3) > { > if (int1 != 32) > return 3; > else if (int2 != 16) > return 2; > else if (int3 != 8) > return 1; > else > return 0; > > } This 3rd choice for form gets my vote too. Embedding the else-if logic in the RETURN is less clear, and more prone to later editing errors, and nesting if/elses does not make the real priority tree clear - and will likely be slower/larger, if that matters. One-exit 'rules' make less sense the lower level your code is. Indeed, I've just speed-optimized a routine, by making the most frequent test, the earliest exit. This only mattered in one small function, called the most. -jg
From: Vladimir Vassilevsky on 7 Jun 2010 16:33 Spehro Pefhany wrote: > On Mon, 7 Jun 2010 06:27:37 -0700 (PDT), rickman <gnuarm(a)gmail.com> > wrote: > > >>There is one other way to write this that I tend to use when coding. >>One of the "guides" to coding I read many years ago, perhaps in >>college said the smaller of the two choices in an if then else should >>be first. This makes it easier for the eye to scan the code and glean >>structure from the shape... a bit like the way I learned to read... >>look for the outline of the words rather than parsing every letter. >> >>Also,, it is not uncommon for the else and the if to be put on the >>same line to show that the "if" is the only statement within the >>else. The result is very clean, simple and uses much less space >>streamlining the code to the point that the structure is very clear to >>the eye. Oh, and you don't need all those parens for single >>statements. >> >>uint16_t Func1(uint16_t int1, uint16_t int2, uint16_t int3) >>{ >> if (int1 != 32) >> return 3; >> else if (int2 != 16) >> return 2; >> else if (int3 != 8) >> return 1; >> else >> return 0; >>} >> >>Rick > > > Why waste all those linefeeds when you can simply write > > uint16_t func(uint16_t i1, uint16_t i2, uint16_t i3) > {return i1 != 32 ? 3: i2 != 16 ? 2: i3 !=8 ? 1 : 0;} This is a BINARY LOGICAL function. No need for if-then-elses. :-) //--------------------- static inline u16 eq(const u16 x, const u16 param) { u16 foo; return x&((((foo|=(foo|=(foo|=(foo|=(foo=x&(~(1<<param)))>>8)>>4)>>2)>>1)&1)^1)<<param); } u16 fubar(u16 x0, u16 x1, u16 x2) { u16 v = eq(x0,5); u16 u = (v>>1)&eq(x1,4); return ((u^16)|((v>>2)^8)|(u>>1)&(eq(x2,3)^8))>>3; } //-------------------------------- Vladimir Vassilevsky DSP and Mixed Signal Design Consultant http://www.abvolt.com
From: Jon Kirwan on 7 Jun 2010 16:45
On Mon, 7 Jun 2010 01:49:13 -0700 (PDT), kishor wrote: >I have some doubt in using "if" "else" statements. > >uint16_t Func1(uint16_t int1, uint16_t int2, uint16_t int3) >{ > > if(int1 == 32) > { > if(int2 == 16) > { > if(int3 == 8) > { > return 0; > } > else > { > return 1; > } > } > else > { > return 2; > } > } > else > { > return 3; > } >} I wouldn't waste so much vertical space on block start and end braces. More like this, instead: uint16_t Func1(uint16_t int1, uint16_t int2, uint16_t int3) { if(int1 == 32) { if(int2 == 16) { if(int3 == 8) { return 0; } else { return 1; } } else { return 2; } } else { return 3; } } That's if I were tempted to keep the braces, which I may be .... or not. If I knew, beforehand, that the only purpose would be to return specific values and that's all... then I might simply eliminate some of them (keeping others to ensure readability as to which parts matched with which.) uint16_t Func1(uint16_t int1, uint16_t int2, uint16_t int3) { if ( int1 == 32 ) { if ( int2 == 16 ) { if ( int3 == 8 ) return 0; else return 1; } else return 2; } else return 3; } I tend to dislike if..else.. statements as shown above. For example, although a close reading of the code does tell me that if int1==32 and int2==16 does always return an explicit value, a cursory and not-so-close look at the inner conditional instead looks to me as though it could "fall through." In other words, when skimming the code quickly I might have to stop myself for a moment and read some more of the code to understand that the inner if cannot fall out and wind up at the bottom of the function where no return statement and explicit value is provided, at all. This also applies to the outer if. I like to see that a function returning a value _always_ has an unconditional return present with an explicit result being provided. Makes me feel safer on first blush, even if a closer reading would also inform me. That said, I would probably "improve" it this way: uint16_t Func1(uint16_t int1, uint16_t int2, uint16_t int3) { if ( int1 == 32 ) { if ( int2 == 16 ) return int3 == 8? 0:1; return 2; } return 3; } It is immediately clear to me that the function must return some explicit value, now. The 'return 3' is unconditional, should everything else fail. And I can easily see that if int1==32, then it must also return some _other_ explicit value... at least 2, if not something else. >uint16_t Func1(uint16_t int1, uint16_t int2, uint16_t int3) >{ > if(int1 != 32) > { > return 3; > } > > if(int2 != 16) > { > return 2; > } > > if(int3 != 8) > { > return 1; > } > > return 0; >} > >Which is the better method (for efficiency, readability or any other >reason)????. Following the above comments I already made, I might recode that as: uint16_t Func1(uint16_t int1, uint16_t int2, uint16_t int3) { if ( int1 != 32 ) return 3; else if ( int2 != 16 ) return 2; return int3 == 8? 0:1; } or as, uint16_t Func1(uint16_t int1, uint16_t int2, uint16_t int3) { if ( int1 != 32 ) return 3; if ( int2 != 16 ) return 2; return int3 == 8? 0:1; } By the time I'm worrying about what exactly the code actually does, I've already noted the return statement doesn't allow continuation and so the choice between consecutive if statements and an if..else is moot, to me. They are the same and similarly readable so I wouldn't care much. Which means I would just be consistent in the coding, then, and do the same elsewhere whichever choice I decided upon. So between the following two cases: uint16_t Func1(uint16_t int1, uint16_t int2, uint16_t int3) { if ( int1 == 32 ) { if ( int2 == 16 ) return int3 == 8? 0:1; return 2; } return 3; } uint16_t Func1(uint16_t int1, uint16_t int2, uint16_t int3) { if ( int1 != 32 ) return 3; if ( int2 != 16 ) return 2; return int3 == 8? 0:1; } I would probably choose the latter one. It is clearer to me. Jon |