I wrote this program, to determine if any given number is happy, and would like to know of any improvements that can be made to make it better. I have tested it, but whether or not it works perfectly I have no idea, there might be some numbers that get through, although I doubt it.
import java.util.HashSet;
import java.util.Scanner;
import java.util.Set;
public class HappyNumber
{
public static void main(String[] args)
{
System.out.print("Please enter a number: ");
int number = new Scanner(System.in).nextInt(), value;
Set<Integer> unique = new HashSet<Integer>();
while (unique.add(number))
{
value = 0;
for (char c : String.valueOf(number).toCharArray())
value += Math.pow(Integer.parseInt(String.valueOf(c)), 2);
number = value;
}
System.out.println(number == 1 ? "Happy" : "Not Happy");
}
}
7 Answers 7
Apart from what @RubberDuck said, you can also do a bit less work by not
converting from/to String
, but iterating over the digits one by one
directly. I've also moved the value
to where it is initialised, that
way its scope is better visible.
After making those changes I arrive at the following snippet. isHappy
or (isHappyNumber
) can be reused and the main
function only does
interface stuff.
import java.util.HashSet;
import java.util.Scanner;
import java.util.Set;
public class HappyNumber
{
public static boolean isHappy(int number)
{
Set<Integer> unique = new HashSet<Integer>();
while (unique.add(number))
{
int value = 0;
while (number > 0)
{
value += Math.pow(number % 10, 2);
number /= 10;
}
number = value;
}
return number == 1;
}
public static void main(String[] args)
{
System.out.print("Please enter a number: ");
int number = new Scanner(System.in).nextInt();
System.out.println(isHappy(number) ? "Happy" : "Not Happy");
}
}
In addition to the point made by RubberDuck there's some stuff you should definitely change in addition to that:
int number = new Scanner(System.in).nextInt(), value;
Don't do this. Every variable declaration has the right to stand on its own. Glossing over this, one could miss the declaration of value
. Additionally if your input is not an int you'll get an InputMismatchException
if the user didn't enter an integer.
There is also a much easier way to get every digit of a number, instead of the bunch of unintuitive String transformations you are doing.
int lowestSignificantDigit = number % 10;
number = number / 10;
You just repeat the code above until there is no number left ;)
Additionally I think that the ternary inside your System.out.println
is rather unclear. As soon as you extract the determination of Happy / Not Happy you should simply use the return value of the method you extracted.
It's a bit of a moot point, but this is not a great way to convert a character digit to integer:
int digit = Integer.parseInt(String.valueOf(c));
This is better:
int digit = c - '0';
Of course this will only work if the value of c
is within the range '0'
to '9'
.
As others have already said, create an boolean isHappy(int)
method to separate its logic from everything else. It will be a nice method with a single responsibility.
Once you have such method, it's good to add unit tests to verify it works correctly:
@Test
public void test_19_Is_Happy() {
assertTrue(isHappy(19));
}
@Test
public void test_22_Is_NotHappy() {
assertFalse(isHappy(22));
}
To be more thorough, you can take the list of known happy numbers from this wikipedia page and turn that into a test:
@Test
public void testKnownHappy() {
Set<Integer> happy = new HashSet<>();
happy.addAll(Arrays.asList(1, 7, 10, 13, 19, 23, 28, 31, 32, 44, 49, 68, 70, 79, 82, 86, 91, 94, 97, 100, 103, 109, 129, 130, 133, 139, 167, 176, 188, 190, 192, 193, 203, 208, 219, 226, 230, 236, 239, 262, 263, 280, 291, 293, 301, 302, 310, 313, 319, 320, 326, 329, 331, 338, 356, 362, 365, 367, 368, 376, 379, 383, 386, 391, 392, 397, 404, 409, 440, 446, 464, 469, 478, 487, 490, 496, 536, 556, 563, 565, 566, 608, 617, 622, 623, 632, 635, 637, 638, 644, 649, 653, 655, 656, 665, 671, 673, 680, 683, 694, 700, 709, 716, 736, 739, 748, 761, 763, 784, 790, 793, 802, 806, 818, 820, 833, 836, 847, 860, 863, 874, 881, 888, 899, 901, 904, 907, 910, 912, 913, 921, 923, 931, 932, 937, 940, 946, 964, 970, 973, 989, 998, 1000));
for (int i = 0; i < 1000; ++i) {
assertEquals(happy.contains(i), isHappy(i));
}
}
-
2\$\begingroup\$ ++ for mentioning proper tests. \$\endgroup\$RubberDuck– RubberDuck2014年11月30日 19:14:00 +00:00Commented Nov 30, 2014 at 19:14
-
\$\begingroup\$
Character.digit(c, 10)
is another alternative toInteger.parseInt(String.valueOf(c))
. \$\endgroup\$200_success– 200_success2014年12月01日 07:32:04 +00:00Commented Dec 1, 2014 at 7:32
The "Happy Number" code is tightly bound to your main routine and user interface. As it is, you couldn't take the code and plug it into another program. It would be best to break this code out into its own class and call it from Main
.
Since all unhappy numbers enter the sequence beginning with 4 there is no need to store the list of numbers. Just test for 4 or 1.
Math.pow()
is an inefficient (and possibly inaccurate) way to square small numbers. It's much better to simply multiply:
for (char c: String.valueOf(number).toCharArray()) {
int digit = Character.digit(c, 10);
value += digit * digit;
}
Of course, this improvement can also be combined with better ways of extracting digits:
for (; number > 0; number /= 10) {
int digit = number % 10;
value += digit * digit;
}
If you're trying to squeeze every last cycle of performance, a ten-element lookup table may be even faster. But profile properly if speed matters so much.
public class HappyNumber {
public static void sumHappy(int number){
int sum =0;
int r =0;
while(number>0){
r = number%10;
sum = sum + (r*r);
number = number/10;
}
if (sum!= 1 && sum > 9){
sumHappy(sum);
}
else if(sum == 1){
System.out.println("Happy number");
}
else{
System.out.println("Not a Happy number");
}
}
public static void main(String[] args) {
int num = 91;
sumHappy(num);
}
}
-
2\$\begingroup\$ Please explain why your solution is better than the one in the question. A code dump like this is not particularly useful. \$\endgroup\$Null– Null2018年10月15日 22:13:03 +00:00Commented Oct 15, 2018 at 22:13
-
2\$\begingroup\$ Welcome to Code Review! Just as @Null stated: You have presented an alternative solution, but haven't reviewed the code. Please explain your reasoning (how your solution works and why it is better than the original) so that the author and other readers can learn from your thought process. Please read Why are alternative solutions not welcome? \$\endgroup\$2018年10月15日 22:22:32 +00:00Commented Oct 15, 2018 at 22:22