Why is an empty else-if statement bad style, and how should I rewrite it?
The program that automatically grades my code is docking me "style-points" for an else-if that doesn't execute any code. It says that it may cause an error, but I don't think it could.
I'm not sure how to change it so that it still works but doesn't break the rule. Why is doing this bad form? I think any other way I write it will be harder for a reader to understand. How should it be written instead?
if (! seesWater(LEFT))
{
turn(LEFT);
}
else if (! seesWater(AHEAD));
else if (! seesWater(RIGHT))
{
turn(RIGHT);
}
else
{
turn180();
}
The reason the else-if is there but does nothing is because of the priority in which I want the code to act:
if (! seesWater(AHEAD))
, then I don't want the rest of the conditions to run at all because they don't matter.
Who says it's "bad style"?
The relevant question to ask is, is this clearer than alternatives? In your specific case, I'd say it is. The code clearly expresses a choice between 4 options, of which one is "do nothing".
The only change I'd make is to replace that rather insignificant semicolon by an empty pair of braces, possibly with a comment to make it clear it's not a mistake.
if (! seesWater(LEFT)) {
turn(LEFT);
}
else if (! seesWater(AHEAD)) {
// nothing required
}
else if (! seesWater(RIGHT)) {
turn(RIGHT);
}
else {
turn180();
}
This is not endorsing 'empty clauses' as a generally-acceptable style; merely that cases should be argued on their merits, not on the basis of some Rule That Must Be Obeyed. It is a matter of developing good taste, and the judgement of taste is for humans, not mindless automata.
If this is the logic you want, there is nothing wrong with your code. In terms of code styling wise, I agree with the other users. Something like this would be clearer in my opinion:
if (! seesWater(LEFT))
{
turn(LEFT);
}
else if (! seesWater(AHEAD))
{
//Do nothing
}
else if (! seesWater(RIGHT))
{
turn(RIGHT);
}
else
{
turn180();
}
But by having priority of making a left turn over moving ahead (by doing nothing), the movement may end up in circles:
If you want the movement to "do nothing" but avoid moving into waters like this:
You may want to change your logic to something like this:
if (! seesWater(AHEAD))
{
//Do nothing. Keep moving
}
else if (! seesWater(LEFT))
{
turn(LEFT);
}
else if (! seesWater(RIGHT))
{
turn(RIGHT);
}
else
{
turn180();
}
You can invert the ! seesWater(AHEAD)
condition. Then you can move the code in its else
clause to its if
clause:
if (! seesWater(LEFT))
{
turn(LEFT);
}
else if (seesWater(AHEAD))
{
if (! seesWater(RIGHT))
{
turn(RIGHT);
}
else
{
turn180();
}
}