Can anybody explain how this line works?
return $y < 0 ? - pip2 : pip2 if $x == 0;
if $y <0
it returns -pip2
, but what it returns when $y >= 0
and $x != 0
?
This line is from this function:
sub _atan {
my( $y, $x ) = @_;
return $y < 0 ? - pip2 : pip2 if $x == 0;
return atan( $y / $x );
}
The postfix "if" means the return statement is only executed if the condition is true, so
return $y < 0 ? - pip2 : pip2 if $x == 0;
is the same as
if ($x == 0)
{
return $y < 0 ? - pip2 : pip2 ;
}
If you're puzzled by the ?: ternary operator, that could be rewritten as a regular if statement too, to yield this
if ($x == 0)
{
if ($y<0)
{
return -pip2;
}
else
{
return pip2;
}
}
its the same as
if($x == 0){
if($y<0){
return -pip2;
}else{
return pip2;
}
}
The entire function then becomes:
sub _atan {
my( $y, $x ) = @_;
if($x == 0){
if($y<0){
return -pip2;
}else{
return pip2;
}
}else{
return atan( $y / $x );
}
}
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.
Too many lines used to solve a problem makes the code difficult to maintain (always obliged to scroll). The solution with nested if is 4 times longer. Imagine working with a screen 4 times smaller. My favorite syntax is:
sub _atan {
my ($y, $x) = @_;
return atan ($y / $x) if $x != 0;
return $y < 0 ? -pip2 : pip2;
}
The benefit of using a postfix operator is reduced if you put them on the next line. This line order (suggested by @daotoad) allows to put the postfix condition on a simpler line.
The initial syntax is also nice, but I would not like to work on code containing the suggested nested if of previous posts.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With