Skip to main content
Code Review

Return to Answer

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

As @Marvin @Marvin and @Pimgd @Pimgd already said, start at the top and work your way down.

As @Marvin and @Pimgd already said, start at the top and work your way down.

As @Marvin and @Pimgd already said, start at the top and work your way down.

Source Link
mdfst13
  • 22.4k
  • 6
  • 34
  • 70

Consider moving your logic into its own method. That will allow for unit testing as well as manual testing in main.

Consider a method for isPalindrome that does a more general check (not just six digits).

 private static final int BASE = 10;
 public static boolean isPalindrome(int number) {
 int reversed = number % BASE;
 number /= BASE;
 while (number > reversed) {
 reversed *= BASE;
 reversed += number % BASE;
 // check for a palindrome with an odd number of digits
 if (number == reversed) {
 return true;
 }
 number /= BASE;
 }
 return number == reversed;
 }

This pulls off the digits from the end and reverses them into a new number. It checks for equality once after putting a digit into the reversed number and before pulling it off the original. That looks for palindromes with an odd number of digits. Once the number is no longer greater than reversed, it checks again to look for palindromes with an even number of digits.

Or break it up into an int collection of digits and compare from start to finish.

 public static boolean isPalindrome(int number) {
 List<Integer> digits = new ArrayList<>();
 while (number > 0) {
 digits.add(number % BASE);
 number /= BASE;
 }
 for (int start = 0, end = digits.size() - 1; start < end; start++, end--) {
 if (digits.get(start) != digits.get(end)) {
 return false;
 }
 }
 return true;
 }

Conceptually simpler but uses more memory and has the List overhead.

As @Marvin and @Pimgd already said, start at the top and work your way down.

Also, do your product > largest check before the isPalindrome check, as it is cheaper.

 public static int find(int ceiling, int floor) {
 int largest = -1;
 for (int i = ceiling; i >= floor; i--) {
 int product = ceiling * i;
 if (product < largest) {
 // all future products will be smaller than this one
 return largest;
 }
 int squared = i * i;
 // if they are large enough to be an answer,
 // we'll check products smaller than i * i later
 // we never need to check products smaller than largest
 int minimum = Math.max(squared, largest);
 do {
 if (isPalindrome(product)) {
 largest = product;
 minimum = Math.max(squared, largest);
 }
 product -= i;
 } while (product >= minimum);
 }
 return largest;
 }

Now we don't have to multiply on each iteration of the inner loop to get product, we multiply once and then decrement from the product.

Then your main would be simpler:

 public static void main(String[] args) {
 System.out.println(LargestPalindromeProduct.find(999, 100));
 }

This would also allow you to set up unit tests. For example, you might check that LargestPalindromeProduct.find(99, 10) is 9009.

lang-java

AltStyle によって変換されたページ (->オリジナル) /