This is a good example of hard to read code.
Let's compare a few different ways to rewrite the code sample and see how we do in retaining brevity and improving readability.
This ternary only version wins for brevity, but it is still hard to read:
sub _atan {
my( $y, $x ) = @_;
return $x == 0 ? ($y < 0 ? -pip2 : pip2)
: atan( $y / $x );
}
I find that chained conditional operators (?:) are only readable when subsequent operators fall in the else position:
sub _atan {
my( $y, $x ) = @_;
return $x != 0 ? atan( $y / $x ) :
$y < 0 ? -pip2 : pip2;
}
Still brief, but readability is improved.
But what about using if
and unless
? Can we have concise, readable code using them, too?
By its nature a straight if/else approach will be more verbose:
sub _atan {
my( $y, $x ) = @_;
my $atan;
if( x == 0 ) {
if( $y < 0 ) {
$atan = -pip2;
}
else {
$atan = pip2;
}
}
else {
$atan = atan( $y / $x )
}
return $atan;
}
It is easy to trace through the above and see what the result will be. So readability wins, but brevity suffers.
I find that using the statement modifier forms of unless
and if
provide a clean way to add short-circuit logic to a chunk of code:
sub _atan {
my( $y, $x ) = @_;
return atan( $y / $x )
unless $x == 0;
return -pip2 if $y < 0;
return pip2;
}
This is consise and readable, but to me it seems like we've got more returns than we need.
So if we introduce a conditional operator to the mix, we get
sub _atan {
my( $y, $x ) = @_;
return atan( $y / $x )
unless $x == 0;
return $y < 0 ? -pip2 : pip2;
}
This form is as concise as any of the above forms, but much easier to understand:
sub _atan {
my( $y, $x ) = @_;
return atan( $y / $x )
unless $x == 0;
return $y < 0 ? -pip2 : pip2;
}
Nested if/else clauses can be difficult to understand. Taking a little care when structuring your decision code can greatly improve readability and therefore maintainability, while retaining concise expression of the underlying logic.
The code smell to be fixed here was the baroque combination of the conditional operator (?:
) with the statement modifier form of if
. By rearranging the order of the tests and carefully choosing how we represent conditional logic, we were able to preserve brevity and clarify the code.