Java Collections Logic / Over coding issue

Question

Below is an implementation of an exercise I've been asked to do (see comments). It works, and the reason I'm posting it is that the function checkMiracle looks like it should be contained in a much smaller loop of code - I'm writing out the same thing plus one ten times. The problem is, I can't seem to find a shorter way of doing it. My question then is can someone point me in any direction of reducing the code in this listing, maybe something to think about that makes it more compact or a 'clever' way of coding it. Any help appreciated. (The exercise sheet is on the JCF so he is forcing us to code this using collections)

/*A 10-digit decimal number N is said to be miraculous if it contains each of the ten decimal digits, and if
the 2-digit number consisting of the first two (most significant, i.e. leftmost) digits of N is divisible by
2, the 3-digit number consisting of the first three digits of N is divisible by 3, and so on up to and including
that N itself is divisible by 10. Write a program to discover a miraculous number (there really is one).
Proceed by making a list of the ten decimal digits, and repeatedly shuffling them until you chance upon an
arrangement that constitutes a miraculous number.
(Note: Type long rather than int is needed for 10-digit decimal integers.) */

import java.util.*;

public class Miracle {

 static private long miracleNum = 0;

 static private ArrayList<Integer> listing = new ArrayList<Integer>();

 static String castValue = "";


 public static void main(String[] args) {

  for(int i = 0; i < 10; i++) listing.add(i); 

  Collections.shuffle(listing);

  while(listing.get(0)==0) Collections.shuffle(listing); //make sure the number doesnt start with zero

  while(!(checkMiracle(listing))) Collections.shuffle(listing);//keep changing it until we get a miracle number



  for(long l : listing) castValue += l;

  miracleNum = Long.parseLong(castValue);

  System.out.println("Miracle num: " + miracleNum);


 }


 static public boolean checkMiracle(ArrayList<Integer> l) {


  long checkValue = Long.parseLong("" + l.get(0) + l.get(1));

  if(checkValue %2 != 0) return false;


  checkValue = Long.parseLong("" + l.get(0) + l.get(1) + l.get(2));

  if(checkValue %3 != 0) return false;


  checkValue = Long.parseLong("" + l.get(0) + l.get(1) + l.get(2) + l.get(3));

  if(checkValue %4 !=0) return false;


  checkValue = Long.parseLong("" + l.get(0) + l.get(1) + l.get(2) + l.get(3) + l.get(4));

  if(checkValue %5 !=0) return false;


  checkValue = Long.parseLong("" + l.get(0) + l.get(1) + l.get(2) + l.get(3) + l.get(4) + l.get(5));

  if(checkValue %6 !=0) return false;


  checkValue = Long.parseLong("" + l.get(0) + l.get(1) + l.get(2) + l.get(3) + l.get(4) + l.get(5) + l.get(6));

  if(checkValue %7 !=0) return false;


  checkValue = Long.parseLong("" + l.get(0) + l.get(1) + l.get(2) + l.get(3) + l.get(4) + l.get(5) + l.get(6) + l.get(7));

  if(checkValue %8 !=0) return false;


  checkValue = Long.parseLong("" + l.get(0) + l.get(1) + l.get(2) + l.get(3) + l.get(4) + l.get(5) + l.get(6) + l.get(7)+ l.get(8));

  if(checkValue %9 !=0) return false;


  checkValue = Long.parseLong("" + l.get(0) + l.get(1) + l.get(2) + l.get(3) + l.get(4) + l.get(5) + l.get(6) + l.get(7)+ l.get(8) + l.get(9));

  if(checkValue %10 !=0) return false;


  return true;


 }


}

Answer 1

Maybe remove some code duplication using loops:

private static boolean checkMiracleN(List<Integer> l, int n){
   long sum = 0;
   for (int i=0; i<n; i++)
       sum = sum * 10 + l.get(i);
   return sum % n == 0;
}

private static boolean checkMiracle(ArrayList<Integer> l){
    for (int n=2; n<=10; n++)
       if (!checkMiracleN(l, n) 
          return false;
    return true;
}

Answer 2

why don't you gather all the conditions inside a loop? Then you can just add next digit to the number to calculate next one.

String partial = Long.parseLong(l.get(0));
for (int i = 1; i < 10; ++i) {
  partial += Long.parseLong(l.get(i));
  if (Long.valueOf(partial) % (i+1) != 0)
    return false;
}
return true;

in addition you could avoid using strings by using exponentiation digit^(displacement). Having to solve a similar problem by using collections seems groesque..

Answer 3

Use a helper function so that you can replace repeated code with a loop:

static public String GetNumberString(ArrayList<Integer> l, int numDigits)
{
    StringBuilder sb = new StringBuilder();

    for(int i = 0; i < numDigits; i++)
    {
        sb.Append(l.get(i));
    }
    return sb.ToString();
}


static public boolean checkMiracle(ArrayList<Integer> l) {

  long checkValue = 0;

  for (int i = 2; i < 10; i++)
  {
      checkValue = Long.parseLong(GetNumberString(l, i));
      if(checkValue % i != 0) return false;
  }
}

This still means that you're building a very similar string each time. An improvement would be to incrementally build the number on each loop iteration, instead of rebuilding it each time.

Answer 4

A couple of short cuts you can make. e.g. if a number is even its last digit must be even

if (l.get(1) % 2 == 0) return false;

if a number is a multiple of ten, its last digit must be a '0'

if (l.get(9) == 0) return false;

If a number is a multiple if 3, the sum if its digits are multiple of three (same for 9)

If a number is a multiple of 5, its last digit must be a 5 or 0.

In most cases you don't need to * or a %. You definitley don't need to create a String and parse it.