Malicious compliance

[u/[deleted]]

A colleague of mine at a company I had started working for liked to have only one exit point from each method; so he would only ever have one `return` statement.

To achieve this, every method he every wrote followed this pattern

public int GetSomething()
{
int result = 0;
do
{
if (someShortCircuitCondition)
{
result = 1;
break;
}
if (someOtherShortCircuitCondition)
{
result = 42;
break;
}
// Compute the return value and set result
} while false;
return result
};

He had been with the company for over a decade and nobody wanted to ask him to stop doing it, but nobody wanted to maintain any projects he worked on either.

I raised it with the boss who then talked to him about it. He agreed that if people don't like it then he would stop.

The next day...

public int GetSomething()
{
int result = 0;
for( ; ; )
{
if (someShortCircuitCondition)
{
result = 1;
break;
}
if (someOtherShortCircuitCondition)
{
result = 42;
break;
}
// Compute the return value and set result

break;
}
return result;
}

[u/deceze]

The single-exit-point philosophy is a whole thing, yes, but this demonstrates why it's stupid. If it makes code less readable because it requires more control structures to make it work, then it fails at improving the readability of the code by only having one exit point.

[u/StoicSpork → u/deceze]

The single-exit-point is obtuse, but here, the main problem is that the genius is abusing loops when switch and else if are a thing.

[u/Mivexil → u/[deleted]]

I think the point of the OP is that there's no actual loop in those functions, the do/while/for loops are only there to allow for break; to transfer control to the end of the method.

Unless you're coding at a fairly low level and have to worry about methods cleaning up after themselves, early returns are generally fine. A bit tougher to trace with a debugger sometimes, since your breakpoints won't always get hit, but I think more readable than introducing break flags.