reverse gets called twice in every step
In every step of the process to finding a palindromic number,
the reverse method gets called twice:
while (!isPalindromic(currentNum)) {
currentNum += reverse(currentNum);
reverseCount++;
}
private static boolean isPalindromic(int s) {
return s == reverse(s);
}
First it gets called in the while loop's condition statement,
then again inside the loop body.
This is a waste,
it would be better to refactor to only call once per iteration.
Instead of reverseCount,
the name stepCount would seem slightly more appropriate,
but this may be just a matter of taste.
isPalindromic is simple but lazy
Although the implementation of isPalindromic is sweet and simple,
it's wasteful and inaccurate.
It involves converting the number to String, reversing, and then converting back to integer.
A more efficient way would be to compare the digits of position \$i\$ and \$n - i - 1\$, until you reach position \$n/2\$.
The method will not work with negative numbers because reverse will not work with negative numbers.
Here's an implementation using math, that works with negative numbers too:
private static boolean isPalindromic(int num) {
int digits = (int) Math.log10(Math.abs(num)) + 1;
int firstDivisor = (int) Math.pow(10, digits - 1);
int lastDivisor = 1;
while (firstDivisor > lastDivisor) {
int firstDigit = num / firstDivisor % 10;
int lastDigit = num / lastDivisor % 10;
if (firstDigit != lastDigit) {
return false;
}
firstDivisor /= 10;
lastDivisor *= 10;
}
return true;
}
And some unit tests to go with it:
@Test
public void testPalindromic_3() {
assertTrue(isPalindromic(3));
}
@Test
public void testPalindromic_13() {
assertFalse(isPalindromic(13));
}
@Test
public void testPalindromic_22() {
assertTrue(isPalindromic(22));
}
@Test
public void testPalindromic_333() {
assertTrue(isPalindromic(333));
}
@Test
public void testPalindromic_1991() {
assertTrue(isPalindromic(1991));
}
@Test
public void testPalindromic_334() {
assertFalse(isPalindromic(334));
}
@Test
public void testPalindromic_minus12321() {
assertTrue(isPalindromic(-12321));
}
reverse is simple but lazy
As mentioned above,
the simple but lazy implementation breaks on negative numbers.
It also breaks on numbers that cause integer overflow when reversed.
And like with isPalindromic,
the logic is wasteful.
It would be better to implement this too using basic math logic:
private static int reverse(int num) {
long reversed = 0;
int work = num;
while (work != 0) {
reversed *= 10;
reversed += work % 10;
if (reversed > Integer.MAX_VALUE) {
throw new IllegalArgumentException("Reversing this number will cause integer overflow: " + num);
}
work /= 10;
}
return (int) reversed;
}
Notice the check for integer overflow in the middle.
It's a mistake of the caller to try to reverse an int that would turn out to be too big.
As with any non-trivial implementation, it's important to have unit tests:
@Test
public void testReverse_3() {
assertEquals(3, reverse(3));
}
@Test
public void testReverse_34() {
assertEquals(43, reverse(34));
}
@Test
public void testReverse_41() {
assertEquals(14, reverse(41));
}
@Test
public void testReverse_123() {
assertEquals(321, reverse(123));
}
@Test
public void testReverse_100() {
assertEquals(1, reverse(100));
}
@Test
public void testReverse_123456789() {
assertEquals(987654321, reverse(123456789));
}
@Test(expected = IllegalArgumentException.class)
public void testReverse_1234567899() {
reverse(1234567899);
}
getPalindrome
As others already pointed out,
getPalindrome is not a great name for this method.
It's also not great that it takes a String and converts it to integer,
because it's an additional responsibility,
so it violates the Single Responsibility Principle.
This way of declaring variables looks odd:
int reverseCount = 0,
currentNum = Integer.parseInt(line)
;
I'd recommend to just stick to traditional style:
int reverseCount = 0;
int currentNum = Integer.parseInt(line);
As with anything non-trivial,
it's extremely helpful to have unit tests.
I also included test cases suggested by @thepace.
I renamed the method to stepsToPalindromic and made it take an integer parameter instead of String.
@Test
public void testStepsToPalindromic_8() {
assertEquals("0 8", stepsToPalindromic(8));
}
@Test
public void testStepsToPalindromic_125() {
assertEquals("1 646", stepsToPalindromic(125));
}
@Test
public void testStepsToPalindromic_195() {
assertEquals("4 9339", stepsToPalindromic(195));
}
@Test
public void testStepsToPalindromic_0() {
assertEquals("0 0", stepsToPalindromic(0));
}
@Test(expected = IllegalArgumentException.class)
public void testStepsToPalindromic_max() {
stepsToPalindromic(Integer.MAX_VALUE);
}
@Test
public void testStepsToPalindromic_minus1() {
assertEquals("0 -1", stepsToPalindromic(-1));
}
@Test
public void testStepsToPalindromic_minus195() {
assertEquals("4 -9339", stepsToPalindromic(-195));
}
@Test
public void testStepsToPalindromic_9998() {
assertEquals("6 8836388", stepsToPalindromic(9998));
}
