0
\$\begingroup\$

I have seen one algorithmic task and I am not sure if my solution is fully efficient. As input to the method you are given positive integer N. And you should return the sum of values of the digits in a decimal representation of N!.

For example, given N = 6 the function should return 9, because N != 720 and sum of digits 7 + 2 + 0 = 9. Result cannot exceeds 100000000. N is higher or equal than 1 and less or equal than 2000.

Is this an efficient solution and the best optimized?

public int solution(int N) {
    long factorial = 1;
    for (int i = 1; i <= N; i++)
        factorial = factorial * i;
    long res = 0;

    while(factorial>0) {
        res+=factorial%10;
        factorial/=10;
    }
    if (res > 100000000)
        return -1;
    return (int) res;
}
\$\endgroup\$

closed as off-topic by 200_success May 19 '15 at 22:27

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. After the question has been edited to contain working code, we will consider reopening it." – 200_success
If this question can be reworded to fit the rules in the help center, please edit the question.

2
\$\begingroup\$

Your code only works for small values of N. The algorighm is effective, but unfortunately your overflows are real.

Factorial numbers get very big, very fast... I think it is around 12! that you run out of space in a 32-bit integer, and about 20! that your run out of space in a 64-bit long.

You need to convert your logic to use BigInteger instead of Long for it to work at all.

\$\endgroup\$

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