HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Counting perfect squares

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
perfectcountingsquares

Problem

Below is the code I have written for counting perfect squares between a given lower and upper bound.

I am using the following concept to solve this:


Starting with 1, there are \$\sqrt{m}\$ square numbers up to and including \$m\$.

public static int CountPerfectSquares(long A, long B)
{            
    int count = 0;

    //Proceed if lowerbound is less than upperbound
    if (A < B)
    {
        //negative numbers are not perfect squares
        if (A < 0 && B < 0) return count; 

        //Reset lowerbound if required
        if (A <= 0) A = 1;

        //Find number of squares between A & B (INCLUSIVE lower and upperbound)
        //count = (int)Math.Sqrt(B) - (int)Math.Sqrt(A);  //Giving wrong count when LowerBound value is a whole square (For exp. CountPerfectSquares(1, 25))

        int sqrtA = (int)Math.Sqrt(A);
        int sqrtB = (int)Math.Sqrt(B);
        count = sqrtB - sqrtA;
        //To handle the case when lowerbound value is a perfect square number
        if (A == sqrtA * sqrtA) 
        {
            count++;
        }
    }

    return count;
}


Can you please help me make this more efficient, if possible?

Is there any other way of handling the case when the lower bound value is a perfect square number, so that I don't have to do an extra count++?

Solution

If A was not included in the observed range, you wouldn't need the if (A == sqrtA * sqrtA) part, right? So, let's use that: instead of checking the range {A,A+1,...,B}, check the range {A-1,A,...,B}. Since you're working with integers, the two are the same:

int sqrtA = (int)Math.Sqrt(A-1);
        int sqrtB = (int)Math.Sqrt(B);
        count = sqrtB - sqrtA;


A minor detail: when returning a predefined constant, I prefer writing the constant itself, not a variable initialized to that value:

if (A < 0 && B < 0) return 0;


It seems more readable to me.

Also, I would replace two main if-s with if (B < 0 || B < A) return 0. This gives the same result, but in a cleaner way.

So, my version would be:

public static int CountPerfectSquares(long A, long B)
{            
    // Return zero if there are obviously no perfect squares
    if (B < 0 || B < A) return 0;

    // Reset the lower bound if required
    if (A <= 0) A = 1;

    // Find the number of perfect squares between A & B (INCLUSIVE)
    return (int)Math.Sqrt(B) - (int)Math.Sqrt(A-1);  // Now giving the correct count, even when the lower bound value is a whole square (for exp. CountPerfectSquares(1, 25))
}

Code Snippets

int sqrtA = (int)Math.Sqrt(A-1);
        int sqrtB = (int)Math.Sqrt(B);
        count = sqrtB - sqrtA;
if (A < 0 && B < 0) return 0;
public static int CountPerfectSquares(long A, long B)
{            
    // Return zero if there are obviously no perfect squares
    if (B < 0 || B < A) return 0;

    // Reset the lower bound if required
    if (A <= 0) A = 1;

    // Find the number of perfect squares between A & B (INCLUSIVE)
    return (int)Math.Sqrt(B) - (int)Math.Sqrt(A-1);  // Now giving the correct count, even when the lower bound value is a whole square (for exp. CountPerfectSquares(1, 25))
}

Context

StackExchange Code Review Q#32751, answer score: 6

Revisions (0)

No revisions yet.