views:

204

answers:

3

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 );
}
+7  A: 

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 );
  }
}
Marius
+16  A: 

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;
    }
}
Paul Dixon
I would probably write it using `unless($x){ ...`
Brad Gilbert
It's the conditional operator, not the ternary operator. :)
Ether
I just implied it was *a* ternary operator, not *the* ternary operator :)
Paul Dixon
@Brad Gilbert be careful `unless($x)` and `if ($x == 0)` have different meanings. `unless($x)` will execute when `$x` is `undef`, `0`, `0.0`, `""`, and `"0"`. `if ($x == 0)` will execute when $x is is `0`, `0.0`, and any string that does not pass `look_like_number` or evaluates to 0 (such as `"0e0"`).
Chas. Owens
`unless(0+$x){` then.
Brad Gilbert
+3  A: 

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.

daotoad
+1 excellent answer
friedo