CODEWITHSUNDEEP
javaalgorithm
Robert Plant
Robert Plant

Reputation: 67

Is this an infinite loop? What am I doing wrong? (Java method)

I'm supposed to write a "bisquare" method that returns the number of bisquares in a range of numbers. I thought I'd figured it out, but when I run this code, nothing displays and my laptop begins to whir like crazy. The system never says it finished.

Here is my code. What am I doing wrong? (I am also looking for a solution to the problem if I am not setting it up correctly.) 

   // An integer that is the sum of the squares of two other integers is called bisquare
   // 5 is bisquare because it is 1*1 + 2*2
   // 4 is bisquare because it is 0*0 + 2*2  (we can use 0 as one of our numbers)
   // 8 is bisquare because it is 2*2 + 2*2  (we can use the same two numbers)
   // 3 is not bisquare, 6 is not bisquare
   //  
   // Given two int parameters, low and high, return the number of bisquares that
   // fall between low and high (inclusive)
   // 
   // EXAMPLES:
   // low = 1, high = 6
   // return 4
   // 1, 2, 4, and 5 are bisquare.  3 and 6 are not
   //
   // low = 7, high = 7
   // return 0
   // 7 is not bisquare.  that is the entire range we are checking. 

   public static int bisquare(int low, int high)
   {      
      int count = 0;
      boolean isBisquare = false;
      for (int checkNum = low; checkNum < high; checkNum++) {
         while (!isBisquare) {
            for (int i = 0; i < high; i++) {
               for (int j = 0; j < high; j++) {
                  if ((i*i) + (j*j) == low) {
                     count++;
                     isBisquare = true;
                  }
               }
            }   
         }
      }
      return count;
   }

Upvotes: 1

Views: 242

Answers (5)

Void_Method
Void_Method

Reputation: 1

I've tried some of the above solutions and I believe I have found an error in all of the above solutions. In the all of the for loops, checkNum should be less than or equal to high, and i or j should be less than or equal to checkNum.

Making these changes will give the answer 7 when given a high of 10 and a low of 1. without these changes, the answer is 5 given the same inputs.

It don't think it will ever count actual high and low values unless this change is made.

Upvotes: 0

Oebele
Oebele

Reputation: 591

There are a few things wrong with your code.

The first thing is the loop

while (!isBisquare) {
    // some code
}

The code in this loop is executed in precisely the same way each time, so if no bisquare is found the first time the code in the loop executes, biSquare is not set to true and will not be set to true in further iterations, leading to an infinite loop.

The second problem is this line:

if ((i*i) + (j*j) == low) {

I think this should be

if ((i*i) + (j*j) == checkNum) {

otherwise you always check for the lowest number in the range whether it is a bisquare.

Combine these two errors and you get an infinite loop whenever the argument low is not a bisquare, regardless of the value of high.

EDIT: Initially I didn't notice what you meant to do with the while loop. After reading some of the discussion I do realize it was to prevent counting one number multiple times. I suggest using @Clashsoft's second solution. This makes the code much more readable and reusable.

Upvotes: 2

Moazzam
Moazzam

Reputation: 429

Your logic was not correct. Below is the correct one:

    public static int bisquare(int low, int high) {
        int count = 0;
        boolean isBisquare = false;
        for (int checkNum = low; checkNum < high; checkNum++) {
            for (int i = 0; i < checkNum && !isBisquare; i++) {
                for (int j = 0; j < checkNum && !isBisquare; j++) {
                    if (((i * i) + (j * j)) == checkNum) {
                        count++;
                        isBisquare = true;
                    }
                }
            }
            isBisquare = false;
        }
        return count;
    }

Upvotes: 0

Clashsoft
Clashsoft

Reputation: 11882

You are not using the checkNum variable correctly. It should be used in the inner two for loops. Also, the while loop is unnecessary and creates an infinite loop for numbers that are not bisquares.

public static int bisquare(int low, int high)
{      
    int count = 0;
    for (int checkNum = low; checkNum < high; checkNum++)
    {
        outerloop:
        for (int i = 0; i < checkNum; i++)
        {
            for (int j = 0; j < checkNum; j++)
            {
                if (i * i + j * j == checkNum)
                {
                    count++;
                    break outerloop;
                }
            }
        }
    }
    return count;
}

For reasons of clarity, you should probably also consider creating a isBisquare(int) method like this:

public static boolean isBisquare(int n)
{
    for (int i = 0; i < n; i++)
    {
        for (int j = 0; j < n; j++)
        {
            if (i * i + j * j == n)
            {
                return true;
            }
        }
    }
    return false;
}

The bisquare method (which should have a better name, say countBisquares) now looks like this:

public static int countBisquares(int low, int high)
{      
    int count = 0;
    for (int checkNum = low; checkNum < high; checkNum++)
    {
        if (isBisquare(checkNum))
        {
            count++;
        }
    }
    return count;
}

Upvotes: 1

Sorin
Sorin

Reputation: 11968

Yes, If none of the (i*i) + (j*j) == low evaluates to true the while will loop infinitely.

Upvotes: 4

Related Questions