0

I've been working on a problem to find whether or not a given integer n is a perfect square. Although the algorithm works, I get a MemoryError. How should I rephrase this code bit?

Thanks in advance.

def is_square(n):    
    for i in range(1, (n/2)):
        i += 1
        if n % i == 0 and n // i == i:
            return True
    return False
Cœur
  • 37,241
  • 25
  • 195
  • 267
cerealdinner
  • 19
  • 2
  • 6
    First, if this is python2, try changing `range` to `xrange`. The former will generate a list of all integers between 1 and n/2 and hold them all in memory. – Eric Appelt May 28 '15 at 21:35
  • Can't tell what is wrong without the input that is causing it. FYI, you only need to check `while i * i <= n` where 'i' starts at 1. Btw why do you increase i by 1 inside the for statement? – marcadian May 28 '15 at 21:42
  • Thanks @eric-appelt, the xrange did the trick! – cerealdinner May 28 '15 at 21:47
  • @marcadian, I increased i by 1 so that it would start at 1 instead of 0. – cerealdinner May 28 '15 at 21:48
  • 1
    Why not have your `range` start at 2? That is the first factor worth checking. Then you won't need a bizarre `i += 1` at the start or your loop. – khelwood May 28 '15 at 21:55

1 Answers1

3

A couple things:

  1. This appears to be Python2 code (since range(1, n/2) would throw a TypeError in Python3 for all odd ns). You should use xrange instead of range as Eric Appelt suggests in the comments. range in Python2 creates a list, when you only really need a generator.

  2. You can cut down on the number of operations you're doing by checking if i * i == n (or i**2 == n). Then you're doing one multiplication (or exponent) and an equality check, instead of a mod, a floordiv, and two equality checks.

  3. If you're going that far, why not just do def is_square(n): root = n**0.5; return int(root) == root? You'll find that a lot of "improving your algorithms" is leveraging math instead of brute force.

Unrelated, there's no reason to do i += 1 inside the for loop. That's what a for loop DOES.

Adam Smith
  • 52,157
  • 12
  • 73
  • 112
  • Thanks Adam. Testing the direct value is a lot faster than testing so many numbers (after all, I'm not looking for factors, just the square)! Also, you're completely on point with #2. Smart, smart! – cerealdinner May 28 '15 at 22:18