7
\$\begingroup\$

This is a practice question:

A happy number is a number defined by the following process:

Starting with any positive integer, replace the number by the sum of the squares of its digits, and repeat the process until the number equals 1 (where it will stay), or it loops endlessly in a cycle which does not include 1. Those numbers for which this process ends in 1 are happy numbers.

Example: 19 is a happy number

\$1^2 + 9^2 = 82\$

\$8^2 + 2^2 = 68\$

\$6^2 + 8^2 = 100\$

\$1^2 + 0^2 + 0^2 = 1\$

Program below solves this problem:

public static boolean isHappy(int n) {
   int sum=0;
   Set<Integer> seenSet = new HashSet<Integer>();
    while (n > 0) {
        int x = n%10;
        sum+=x*x;
        n=n/10;

        if (n == 0 && sum != 1) {
            n=sum;
            sum=0;
            seenSet.add(n);
        } else if (n== 0 && sum == 1 ) {
            return true;
        }
            else if (seenSet.contains(n)) {
            return false;
        }
    }

    return true;
}

I'm looking for any comments on how to improve the code, but a specific focus on whether the way the method returns from multiple places would be appreciated - the multiple return statements concern me.

\$\endgroup\$
6
\$\begingroup\$

The repeated n == 0 conditions are not pretty. It would be better to turn that part into a nested condition to avoid the duplication. It will also help simplifying the two conditions on sum.

In addition, there is one more comparison with 0 of n: in the loop condition. That's unnecessary, because n will never become 0. Only if the input is 0, but then the current implementation will return true, which is wrong, since 0 is not a happy number. As such, the loop can be while (true) infinite loop, and your could replace the last return statement with throw new IllegalStateException("whaaaat? How did you get here? ") or similar.

The overall logic would be easier to follow if you separate the summing of the digits from the method exit control. You could add a helper function that returns the sum of the squares of digits. Based on that result you could decide to return a value or continue for another round.

You can replace n=n/10 with n /= 10.

The formatting is messy too. Try the auto reformat feature of your favorite IDE.

\$\endgroup\$
  • \$\begingroup\$ changing n = n/10 to n/=10 improves the run time from 224ms to 208ms. \$\endgroup\$ – linoox Sep 15 '15 at 1:49
  • \$\begingroup\$ @linoox I would expect n = n / 10 and n /= 10 to compile to exactly the same code. You can verify that using javap -c. \$\endgroup\$ – 200_success Sep 15 '15 at 2:36
  • \$\begingroup\$ i found that surprising too. I ran it again and this time it took the same as original (224ms). I should have run it multiple times before commenting here and misleading others. It was strange that's why earlier i tried to make similar changes to sum+=x*x by eliminating x and directly substituting but it took longer. The OJ might not be accurate or could be running on VMs with differing configurations. \$\endgroup\$ – linoox Sep 15 '15 at 2:44
  • \$\begingroup\$ @200_success, what about the running time of this solution? just curious. \$\endgroup\$ – CodeYogi Sep 15 '15 at 5:16
  • \$\begingroup\$ runtime on leetcode OJ was 224ms for the posted solution \$\endgroup\$ – linoox Sep 15 '15 at 12:44

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.