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