Prime factors in C

Question

I came across this effective program for printing all the prime factors of a given number:

# include <stdio.h>
# include <math.h>

// A function to print all prime factors of a given number n
void primeFactors(int n)
{
    // Print the number of 2s that divide n
    while (n%2 == 0)
    {
        printf("%d ", 2);
        n = n/2;
    }

    // n must be odd at this point.  So we can skip 
    // one element (Note i = i +2)
    for (int i = 3; i <= sqrt(n); i = i+2)
    {
        // While i divides n, print i and divide n
        while (n%i == 0)
        {
            printf("%d ", i);
            n = n/i;
        }
    }

    // This condition is to handle the case when n 
    // is a prime number greater than 2
    if (n > 2&&n%2!=0)
        printf ("%d ", n);
}

/* Driver program to test above function */
int main()
{
    int n = 315;
    primeFactors(n);
    return 0;
}

The output is 3 3 5 7. That's perfect.

But I have a confusion in this algorithm. The sqrt() returns a float value. If it is displayed in integer format in printf, it returns some random, large number. If this is the case, the condition in the for loop should fail, because sqrt() as an integer returns a random number. Could someone explain this?

Plus,could someone verify this? This algorithm was wrongly written in geeksforgeeks website as if(n>2) instead of if(n>2&&n!=0), which was added by me. Someone please verify this algorithm. Thanks in advance.

If you send the `double` result of `sqrt` to an integer `printf` format you **will** get garbage. — Weather Vane, Jun 08 '18 at 22:07
What you should do, is get the `sqrt` before the loop, and not continually rely on the compiler to optimise that on very iteration. I know that "premature optimisation is the root of all evil" but common sense applies. — Weather Vane, Jun 08 '18 at 22:08
When saying ", displaying the return value of sqrt() function as an integer in prinntf() function" do you mean: `printf("%i", sqrt(93));` ? That's likely to produce garbage, since a double in memory is stored entirely differently than an integer (sign bit, exponent and mantissa for double, two's complement for int), and passing a double value to `printf` with an integer format specifier does not convert the int value to a double, but just take its bit representation and interprets it as double... — Michael Beer, Jun 08 '18 at 22:09
The question has been edited, and it is precise now. @MichaelBeer Kindly clarify me — Subash, Jun 08 '18 at 22:13
In a relational operator expression, the "usual conversions" apply: see pp. 191ff and 162ff in Harbison and Steele (3rd ed). So in `3 < 4.5` both operands are converted to `float`. `sqrt(n)` is indeed a float so you have to print it using a float specifier. — NickD, Jun 08 '18 at 22:15
So, the editied question still says "If it is displayed in integer format in printf, it returns some random, large number. If this is the case, the condition in the for loop should fail, because sqrt() as an integer returns a random number". Well, in the loop, `double` from `sqrt` is automatically cast to `int`. But when you `printf` it as type `%d`, the function **believes** you passed it a value of the right type. — Weather Vane, Jun 08 '18 at 22:17
Please forget `float` and use `double` unless there is a very good reason not to, like you are stuck in 20th century, or the MCU can't handle it. — Weather Vane, Jun 08 '18 at 22:29
So, @Nick the variable i will be converted into float from integer while comparing itself with the return value of sqrt() function. After the comparision is done, the integer i will get back to it's integer value. Is it correct? — Subash, Jun 08 '18 at 22:32
It is `double` not the prehistoric `float`. And `i` is never changed. Have read about `double` yet? — Weather Vane, Jun 08 '18 at 22:34
@Subash: In an expression like your `i < sqrt(n)` the variables `i` and `n` or whatever *never* change their value. When comparing, their values are read, converted if necessary, and compared, but this happens not by altering the content of the variables. The content of the variables remains the same (except you use an operator that *explicitly alters the content of its operands like `++`). — Michael Beer, Jun 08 '18 at 22:44
Once more, since its still not clear to me: Your only problem is, that when printing the value of an `sqrt()` call with an integer format specifier (like `printf("%d", sqrt(315));` ) you get an unexpected output, and hence you assume that your program does not work properly? Then, the answers provided this far should solve your problem all right? the unexpected value is an output problem. — Michael Beer, Jun 08 '18 at 22:47
As @MichaelBeer and the accepted answer state, conversion happens in certain contexts (e.g. when two values have to be compared using a relational operator), but the variables themselves are not touched. And yes, I should have said "double", not "float". — NickD, Jun 08 '18 at 22:57
@Subash: `n>2 && n != 0` is redundant (if `n > 2`, it cannot be equal to 0), but I suppose you meant `n > 2 && n%2 != 0`. However, it is impossible for `n%2` to be 0 at that point, because after the `while (n%2 == 0)` loop, `n` is clearly odd, and dividing it by one (or more) of its factors cannot make it even. So I strongly suggest you remove your incorrect change to the algorithm. — rici, Jun 09 '18 at 03:06
@WeatherVane: In this function the compiler will not optimise `sqrt(n)` to a single computation for the simple reason that `n` is modified during the execution of the loop. The correct solution (mentioned in Keith Thompson's answer, but not prominently enough IMHO) is to pretend math.h doesn't exist and write the loop condition as `i * i <= n`. (If you don't like multiplying either, you can do it with adds: `i = 3, isqrd = 9; isqrd <= n; isqrd += 4*(i + 1), i += 2`.) — rici, Jun 09 '18 at 03:42

Keith Thompson · Accepted Answer · 2018-06-09T22:56:57.910

6

If you try to print the value of sqrt(n) as if it were an integer:

printf("sqrt(n) = %d\n", sqrt(n)); // Don't do this

then you have undefined behavior. sqrt() returns a result of type double. The compiler doesn't know (or isn't required to know) the expected type of the argument. It's up to you to pass an argument of the correct type. The above call to printf has undefined behavior.

In other contexts, where the type and expected type of an expression are unambiguous, the language requires implicit conversions. In particular, in the expression i <= sqrt(n), where i and n are of type int, the argument n is converted from int to double (the parameter type for sqrt()), and the value ofiis also converted frominttodouble`. The result is likely to be what you expected it to be.

Incidentally, this:

for (int i = 3; i <= sqrt(n); i = i+2) ...

is likely to be inefficient. The sqrt function is relatively expensive, and it's going to be called on each iteration of the loop. (Precomputing sqrt(n) is a good solution in some cases, but here the value of n can change within the loop.)

An alternative is:

for (int i = 3; i*i <= n; i += 2) ...

which avoids the complications of floating-point arithmetic (but think about whether i*i can overflow).

(It would be nice if C had an integer square root function in the standard library, but it doesn't.)

edited Jun 09 '18 at 22:56

answered Jun 08 '18 at 22:30

Keith Thompson

254,901
44
429
631

To be on the safe side, it could be `const int sqrt_n = ceil(sqrt(n));` – Weather Vane Jun 08 '18 at 22:38
@WeatherVane I think `floor` not `ceil` – Soner from The Ottoman Empire Jun 08 '18 at 22:40
Oh my god, you simplified the algorithm and computational load very efficiently. Thanks a lot, @Keith Thompson. I still have one question. What if I do the following: int<=float ; float<=int; In general, what will happen if I compare two different data types, as a condition to exit the loop? – Subash Jun 08 '18 at 22:40
@snr `ceil` would exclude rounding errors. The conversion from `double `to `int` would `floor` it, I suggest raising it. – Weather Vane Jun 08 '18 at 22:40
@Subash: the comparison `int <= float` or `float <= int` is treated to the normal promotions, so the `int` is converted to `float` and the comparison is done using two `float` operands. – Jonathan Leffler Jun 08 '18 at 22:43
There are plenty of relatively simple algorithms for computing integer square root. It is not difficult to code them up. – Peter Jun 08 '18 at 23:24
1

Keith: the optimisation of precomputing sqrt changes the semantics of the test because `n` is not constant through the loop. That will cause excessive work. The correct test is the `i*i <= n` alternative. – rici Jun 09 '18 at 02:56
As an example, consider the case of calling the function with 1,162,261,427. The original will print 19 3's and stop immediately. With sqrt(n) cached, the program will check to see if `1` can be divided by each of the odd numbers from 5 to 34091. – rici Jun 09 '18 at 03:33
`i*i` in `int i = 3; i*i <= n` may overflow. `int i = 3; i <= n/i` does not. – chux - Reinstate Monica Aug 06 '18 at 17:22

Soner from The Ottoman Empire · Answer 2 · 2018-06-08T22:52:30.977

As far as I understand from your question that you are passing float to printf() function with %d specifier which is for integer. It's undefined behavior.

N1570 7.21.6.1 paragraph 9:

... If any argument is not the correct type for the corresponding conversion specification, the behavior is undefined.

That's why you see the garbage value. The compiler may warn you but not error either compile time or run-time since C is not strictly typed language.

It's compiled successfully but its output is -376018152. So again, it's UB.

score 0 · Answer 3 · answered Jun 09 '18 at 10:36

Others have suggested pre-calculating the integer square root, but you need to be careful to recalculate it as needed. For the main loop I would use something like:

// n must be odd at this point.  So we can skip 
// one element (Note i = i +2)
int limit = isqrt(n);  // Calculate integer square root.
for (int i = 3; i <= limit; i = i+2)
{
    // While i divides n, print i and divide n
    if (n % i == 0)
    {
        printf("%d ", i);
        n = n / i;

        while (n%i == 0)
        {
            printf("%d ", i);
            n = n / i;
        }

        // Recalculate limit as n has changed.
        limit = isqrt(n);
    }
}

This only recalculates the square root when the number being tested has changed. I use isqrt(n) to indicate the function to do the actual calculation. In my own code I use Newton-Raphson, but there are other possible methods.

Your test at the end could be simplified to:

// This condition is to handle the case when n 
// is a prime number greater than 2
if (n > 1)
{
    printf ("%d ", n);
}

There is at most one prime factor greater than the square root of a number. That lone factor (if present) will be the remainder after all the smaller prime factors have been divided out.

Prime factors in C

3 Answers3