www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - question about conditional operator (?:)

reply Richard <richardpalme5b gmail.com> writes:
Hello all,

I've got a program that correctly computes the largest factor in 
the prime decomposition of a positive number:

*****************************************************
import std.math std.stdio;

ulong largestPrimeFactor(ulong n) {
     for(ulong p=2; p<=sqrt(cast(real)n); ) {
         if(n%p==0)
             n/=p;
         else
             p+=1;
     }
     return n;
}

void main() {
     writeln(largestPrimeFactor(4));
}
*****************************************************

However, if I replace the content of the for loop with the ?: 
operator, the program is not
correct anymore (largestPrimeFactor(4) now returns 3):

*****************************************************
import std.math std.stdio;

ulong largestPrimeFactor(ulong n) {
     for(ulong p=2; p<=sqrt(cast(real)n); ) {
         n%p==0 ? n/=p : p+=1 ;
     }
     return n;
}

void main() {
     writeln(largestPrimeFactor(4));
}
*****************************************************

What am I doing wrong here?
I'm using dmd version 2.071.1-0 on ubuntu.
Jul 26 2016
next sibling parent Cauterite <cauterite gmail.com> writes:
On Tuesday, 26 July 2016 at 13:09:28 UTC, Richard wrote:

 Hello all,


try using some parentheses:
(n%p==0) ? (n/=p) : (p+=1) ;
Jul 26 2016
prev sibling next sibling parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Tuesday, 26 July 2016 at 13:09:28 UTC, Richard wrote:

         n%p==0 ? n/=p : p+=1 ;



Try

(n%p==0) ? n/=p : p+=1 ;


I'm pretty sure the order of operations (precedence rules) puts 
?: above ==.
Jul 26 2016
parent Adam D. Ruppe <destructionator gmail.com> writes:
On Tuesday, 26 July 2016 at 13:17:27 UTC, Adam D. Ruppe wrote:

 (n%p==0) ? n/=p : p+=1 ;



And actually, using ?: with /= and += is kinda bizarre.

I think you are better off leaving this as if/else since the 
point of this is the assignment rather than the return value.
Jul 26 2016
prev sibling parent reply ag0aep6g <anonymous example.com> writes:
On 07/26/2016 03:09 PM, Richard wrote:

         if(n%p==0)
             n/=p;
         else
             p+=1;


[...]

 However, if I replace the content of the for loop with the ?: operator,
 the program is not
 correct anymore (largestPrimeFactor(4) now returns 3):


[...]

         n%p==0 ? n/=p : p+=1 ;


[...]

 What am I doing wrong here?


Operator precedence is different from what you think. `a ? b : c = d` 
means `(a ? b : c) = d`. But you want `a ? b : (c = d)`. So you need 
parentheses around `p+=1`.

Or just go with `if` and `else`. It's clearer anyway.
Jul 26 2016
parent reply Richard <richardpalme5b gmail.com> writes:
On Tuesday, 26 July 2016 at 13:19:54 UTC, ag0aep6g wrote:

 Operator precedence is different from what you think. `a ? b : 
 c = d` means `(a ? b : c) = d`. But you want `a ? b : (c = d)`. 
 So you need parentheses around `p+=1`.

 Or just go with `if` and `else`. It's clearer anyway.


 From http://wiki.dlang.org/Operator_precedence

Priority(15 is highest)
3                      Conditional operator                  ?: 	
2                      Assignment operators    = -= += <<= >>= 

= = *= %= ^= ^^= ~=






I was actually close to not needing parentheses :). But I see 
that your suggestion to stick with if else in this case is the 
sensible thing to do, especially since ?: seems to lead to more 
errors. Thanks for the answers.
Jul 26 2016
next sibling parent Jonathan M Davis via Digitalmars-d-learn writes:
On Tuesday, July 26, 2016 13:41:39 Richard via Digitalmars-d-learn wrote:

 On Tuesday, 26 July 2016 at 13:19:54 UTC, ag0aep6g wrote:

 Operator precedence is different from what you think. `a ? b :
 c = d` means `(a ? b : c) = d`. But you want `a ? b : (c = d)`.
 So you need parentheses around `p+=1`.

 Or just go with `if` and `else`. It's clearer anyway.


  From http://wiki.dlang.org/Operator_precedence

 Priority(15 is highest)
 3                      Conditional operator                  ?:
 2                      Assignment operators    = -= += <<= >>=

  >>>= = *= %= ^= ^^= ~=

 I was actually close to not needing parentheses :). But I see
 that your suggestion to stick with if else in this case is the
 sensible thing to do, especially since ?: seems to lead to more
 errors. Thanks for the answers.


The ternary operator is invaluable for stuff like initializing a constant
based on boolean condition, and it can be useful in code in general, but I
think that most everyone would agree that you don't want to be doing
mutating operations inside of a ternary operator and that if/else statements
are far better suited to that - particularly since the ternary operator
results in a value, which is not at all what you're looking to do here.

That being said, it surprises me how often folks get confused by the
precedence of the ternary operator and start throwing parens around it. The
only reason that you'd need to here is becasue of the assignment operations,
which really shouldn't be used in a ternary expression anyway (except when
using the result of the whole expression as the value to assign).

- Jonathan M Davis
Jul 26 2016
prev sibling parent =?UTF-8?Q?Ali_=c3=87ehreli?= <acehreli yahoo.com> writes:
On 07/26/2016 06:41 AM, Richard wrote:


 From http://wiki.dlang.org/Operator_precedence


In case it's useful to others, I explain that table a little bit here 
(associativity, unordered operators, and the precedence of =>):

   http://ddili.org/ders/d.en/operator_precedence.html

Ali
Jul 26 2016