7
\$\begingroup\$

I am using mod and my code works. I am wondering if there is a more efficient way to write a method that returns the total number of divisors for an integer.

function getDivisorsCnt(n){
    var divisors = 0;
    mod = n;
    while (mod > 0){
      if(n % mod === 0){
        divisors++;
      }
      mod--;
    }
    return divisors;
}
\$\endgroup\$
1
  • \$\begingroup\$ As always, you can play the speed/memory trade-off and a use a (huge?) pre-computed array of primes. \$\endgroup\$ – GameAlchemist Feb 20 '16 at 22:51
9
\$\begingroup\$

Some general remarks:

  • Variables should be declared explicitly with var, otherwise you create a global variable.
  • The indent amount is different between the first level and the deeper levels.
  • The spacing is not consistent: while ( vs if(. Generally I would insert a space at least after keywords, and around parentheses/braces etc.

Your implementation is correct but not efficient, as the number of loop iterations and remainder operations is equal to the input number.

The count of divisors can be efficiently computed from the prime number factorization: If $$ n = p_1^{e_1} \, p_2^{e_2} \cdots p_k^{e_k} $$ is the factorization of \$ n \$ into prime numbers \$ p_i \$ with exponents \$ e_i \$, then $$ \sigma_0(n) = (e_1+1)(e_2+1) \cdots (e_k+1) $$ is the number of divisors of \$ n \$, see for example Wikipedia: Divisor function. Example: $$ 720 = 2^4 \cdot 3^2 \cdot 5^1 \Longrightarrow \sigma_0(720) = (4+1)(2+1)(1+1) = 30 \, . $$

An implementation in JavaScript would be

function getDivisorsCnt(n){

    var numDivisors = 1;
    var factor = 2; // Candidate for prime factor of `n`

    // If `n` is not a prime number then it must have one factor
    // which is <= `sqrt(n)`, so we try these first:
    while (factor * factor <= n) {
        if (n % factor === 0) {
            // `factor` is a prime factor of `n`, determine the exponent:
            var exponent = 0;
            do {
                n /= factor;
                exponent++;
            } while (n % factor === 0)
            // `factor^exponent` is one term in the prime factorization of n,
            // this contributes as factor `exponent + 1`:
            numDivisors *= exponent + 1;
        }
        // Next possible prime factor:
        factor = factor == 2 ? 3 : factor + 2
    }

    // Now `n` is either 1 or a prime number. In the latter case,
    // it contributes a factor 2:
    if (n > 1) {
        numDivisors *= 2;
    }

    return numDivisors;
}

As an example, getDivisorsCnt(720) requires 720 remainder operations in your algorithm, but only 8 remainder operations and 6 divisions in this algorithm.

\$\endgroup\$
3
  • \$\begingroup\$ At each iteration, you have to calculate factor², wouldn't it be faster to compare factor with √n (calculated only once)? \$\endgroup\$ – Yukulélé Apr 13 '20 at 18:56
  • 1
    \$\begingroup\$ @Yukulélé: No, because n decreases in each loop iteration. \$\endgroup\$ – Martin R Apr 13 '20 at 18:58
  • \$\begingroup\$ Since all primes above 3 are of the form 6n±1 can we improve with something like factor += (k = !k) ? 2 : 4? \$\endgroup\$ – Yukulélé Apr 13 '20 at 20:19
4
\$\begingroup\$

There definitely is a more efficient way to count a value's divisors.

We know that a number's square root multiplied by itself is the largest possible divisor besides itself, so we cut the number of divisors we check in half.

function getDivisorsCount(n) {
    // 1 is a special case where "1 and itself" are only one divisor rather than 2
    if (n === 1) {
        return 1;
    }

    var divisors = 2; // acounts for "1 and itself"

    var mod = 2;
    while (mod * mod <= n) {
        if (n % mod === 0) {
            if (mod * mod < number) {
                // mod and number/mod are (different) divisors
                divisors += 2;
            } 
            else {
                // mod == number/mod is a divisor
                divisors += 1; 
            }
        }
        mod++;
    }

    return divisors;
}

As a note, zero and negative numbers are probably also special cases that aren't necessarily handled correctly here.

\$\endgroup\$
5
  • \$\begingroup\$ I think this it not entirely correct. In your loop, each divisor mod has to be counted twice (because both mod and n/mod are a divisor of n), unless mod*mod == n. A Java implementation of that method is (for example) here: codereview.stackexchange.com/a/74900/35991. \$\endgroup\$ – Martin R Feb 20 '16 at 18:59
  • \$\begingroup\$ 10 has the divisors 1, 2, 5, 10. You initialize divisors with 2 (for 1 and 10). Then mod runs from 2 to 3, and divisors is incremented by one (when mod==2), giving the (incorrect) result 3. Or did I overlook something? \$\endgroup\$ – Martin R Feb 20 '16 at 19:05
  • \$\begingroup\$ You're right. I see it now. Now I kind of wonder what would be more efficient... this approach, or leaving my original loop body and changing the conditional to while (mod * 2 <= n) (and it might depend on which number is passed in). \$\endgroup\$ – nhgrif Feb 20 '16 at 19:07
  • \$\begingroup\$ Wouldn't it better to write that while loop as a for loop? for(var mod = 2; mod*mod <= n; mod++) { \$\endgroup\$ – SirPython Feb 20 '16 at 22:34
  • \$\begingroup\$ Maybe. I don't really know JavaScript... This is more of an algorithm answer... \$\endgroup\$ – nhgrif Feb 20 '16 at 23:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.