I'm having an internal dispute about the right way to do something. Maybe you can help :)
Let's say I have the functions
foo(x, y)
{
if (x satisfies condition c1)
if (y satisfies condition c2)
bar(x)
else
bar(x)
...
}
bar(x)
{
...
}
An alternative to this would be
foo(x, y)
{
doBarFlag = false
if (x satisfies cond开发者_高级运维ition c1)
if (y satisfies condition c2)
doBarFlag = true
else
doBarFlag = true
if (doBarFlag)
{
...code from bar() goes in here
}
...
}
And another alternative (slight variation of above)
foo(x, y)
{
doBarFlag = true
if (x satisfies condition c1)
if (y DOES NOT satisfy condition c2)
doBarFlag = false
if (doBarFlag)
{
...code from bar() goes in here
}
...
}
Assume that bar() is less than ten lines. Which style would you prefer? Why? I'm leaning toward the first example, but I'd like to know what others think.
How about
foo( x, y ) {
if ( ! c1(x) || c2(y) )
bar(x)
}
There's a name for this logic transformation, which I forget…
The other answers have good suggestions on how to improve the code, I just have a styling suggestion. I would have braces on all code blocks regardless of length:
// C# style
foo(x, y)
{
if (condition)
{
// do something
}
else
{
// do something else
}
}
// What I normally use
foo(x, y) {
if (condition) {
// do something
} else {
// do something else
}
}
That way if you want to add another statement later it won't be added in the wrong place. But don't get too religious on styling, just be consistent with whatever you use.
First of all, one of the reasons for defining functions is to be able to refer to (and not have to repeat) a bunch of code, using a short name. Hence, using that short name repeatedly isn't necessarily a bad thing. Thus, your option #1 is perfectly reasonable, and the least redundant.
Second of all, if that's really all that's going on in foo(), why not just do this instead?
foo(x, y)
{
if (x satisfies condition c1) && !(y satisfies condition c2)
return
bar(x)
}
or even just
foo(x, y)
{
if !(x satisfies condition c1) || (y satisfies condition c2)
bar(x)
}
In most languages, the && and || operators (or their equivalents) are short-circuited, and thus won't try to evaluate their right-hand side if they can determine their truth value from just the left. As such, you often don't need to worry about things like "property compared in y only exists if x is true" or the like.
how about
foo(x,y)
{
if(x satisfies c1 && y does not satisfy c2)
return;
bar(x);
}
this way you can easily see that when bar(x) is not executed.
精彩评论