newAtJava
newAtJava

Reputation: 13

Check Values Method Not Working for Checking for a Magic Square

This method is always returning false even when the correct numbers are inputted. If the numbers 1-n^2 and no duplicates are entered then it should return true.

   public boolean checkValues()
{
  int numCounter=1;
  boolean okay=false;
  while (numCounter<=n*n)
  {
     for (int i=0; i< n ; i++) 
     {
        for (int j=0; j< n ; j++)
        {
           if ((int) square.get(i).get(j)==numCounter)
              okay=true;  
        }
     }
     okay=false;
     numCounter++;
  }
  if (numCounter==n*n)
     return true;
  else 
     return false;  
}

Upvotes: 1

Views: 76

Answers (1)

Paul Lo
Paul Lo

Reputation: 6138

Change if (numCounter==n*n) to if (numCounter==n*n+1) since your while-loop finally would do a numCounter++; again, or replace while (numCounter<=n*n) with while(numCounter<n*n) if n*n is not really required to be checked

Updated:

I noticed that you don't really use okay in the end, I think you might need to change to:

public boolean checkValues()
{
  int numCounter=1;
  boolean okay=false;
  while (numCounter<=n*n)
  {
     ok = false;
     for (int i=0; i< n ; i++) 
     {
        for (int j=0; j< n ; j++)
        {
           if ((int) square.get(i).get(j)==numCounter)
              ok=true;  
        }
     }
     if(!ok)  // numCounter cannot be found
         return false;
     numCounter++;
  }
  return true; // successfully passed the check through 1 to n^2 
}

However, I think a better solution in terms of time complexity is as follows, which you can use a HashSet to check duplicates for you.

public boolean checkValues()
{
     Set<Integer> total = new HashSet<Integer>();
     for (int i=0; i< n ; i++){
        for (int j=0; j< n ; j++){
           int num = square.get(i).get(j);
           if(num>=1 && num<=n*n)
               total.add(num);
        }
     }
     if(total.size() == n*n)
        return true;
     else
        return false;
}

Upvotes: 1

Related Questions