Nikaido
Nikaido

Reputation: 4629

Strange behavior with generator\iterator

I got a problem with an iterator which I created for some OOP exercises.

Here is the problematic generator:

def shapeIterator(listOfShapes):
    print("Generator...")
    print(listOfShapes)
    listOfShapessoretedbyArea = shape.sortedByArea(listOfShapes)
    for shapes in listOfShapessoretedbyArea:
        yield str(shapes)

shape.sortedByArea(listOfShapes) is a static method, which need one argument, a list, which is sorted by the calculate area, and returned to the caller. This method works perfectly in this main function:

if __name__ == '__main__':
    rect = rectangle(20, 5)
    squa = square(2)
    tri = equiTria(2, 5)
    circ = circle(2)
    pent = pentagon(5)
    hexa = hexagon(3)
    listOfShapes = [rect, squa, hexa, tri, circ, pent]
    listOfShapessoretedbyArea = sorted(listOfShapes, key=lambda x: x.calculate_area())
    listOfShapessoretedbyPeri = sorted(listOfShapes, key=lambda x: x.calculate_perimeter())
    listOfShapessoretedbyArea2 = shape.sortedByArea(listOfShapes)
    listOfShapessoretedbyPeri2 = shape.sortedByPerim(listOfShapes)
    iterator = shapeIterator(listOfShapes)
    for i in range(6):
        sleep(1)
        value = next(iterator)
        print(value)
    print("NOT SORTED")
    for shape in listOfShapes:
        print(str(shape))
    print("\nSORTED BY AREA")
    for shape in listOfShapessoretedbyArea:
        print(str(shape))
    print("\nSORTED BY PERIMETER")
    for shape in listOfShapessoretedbyPeri:
        print(str(shape))
    print("\nSORTED BY AREA v2")
    for shape in listOfShapessoretedbyArea2:
        print(str(shape))
    print("\nSORTED BY PERIMETER v2")
    for shape in listOfShapessoretedbyPeri2:
        print(str(shape))

but when I move this part:

iterator = shapeIterator(listOfShapes)
for i in range(6):
    sleep(1)
    value = next(iterator)
    print(value)

at the end of the main, like this:

if __name__ == '__main__':
    rect = rectangle(20, 5)
    squa = square(2)
    tri = equiTria(2, 5)
    circ = circle(2)
    pent = pentagon(5)
    hexa = hexagon(3)
    listOfShapes = [rect, squa, hexa, tri, circ, pent]
    listOfShapessoretedbyArea = sorted(listOfShapes, key=lambda x: x.calculate_area())
    listOfShapessoretedbyPeri = sorted(listOfShapes, key=lambda x: x.calculate_perimeter())
    listOfShapessoretedbyArea2 = shape.sortedByArea(listOfShapes)
    listOfShapessoretedbyPeri2 = shape.sortedByPerim(listOfShapes)
    print("NOT SORTED")
    for shape in listOfShapes:
        print(str(shape))
    print("\nSORTED BY AREA")
    for shape in listOfShapessoretedbyArea:
        print(str(shape))
    print("\nSORTED BY PERIMETER")
    for shape in listOfShapessoretedbyPeri:
        print(str(shape))
    print("\nSORTED BY AREA v2")
    for shape in listOfShapessoretedbyArea2:
        print(str(shape))
    print("\nSORTED BY PERIMETER v2")
    for shape in listOfShapessoretedbyPeri2:
        print(str(shape))
    iterator = shapeIterator(listOfShapes)
    for i in range(6):
        sleep(1)
        value = next(iterator)
        print(value)

I got this error:

TypeError: sortedByArea() takes 1 positional argument but 2 were given

That's very strange. Trying to do some naive debug, I printed the argument passed at the function sortedByArea() in the second case, and I got effectively two arguments. One is the to string value printed in the last for each statement, and the second is the list itself.

The last to string value is referred to this for each statement:

for shape in listOfShapessoretedbyPeri2:
    print(str(shape))

I also tried to change the value of the list, and effectively the value "concatenated" to the argument passed to the shapeIterator function is the last string printed.

If needed here are the classes and import used in the main .py:

from math import pi
from math import sqrt
from time import sleep


class shape():

    def calculate_area():
        pass

    def calculate_perimeter():
        pass

    def ltarea(self, other):
        return self.calculate_area() < other.calculate_area()

    def ltperim(self, other):
        return self.calculate_perimeter() < other.calculate_perimeter()

    def sortedByArea(shapes):
        return sorted(shapes, key=lambda x: x.calculate_area())

    def sortedByPerim(shapes):
        return sorted(shapes, key=lambda x: x.calculate_perimeter())

    def nametype(self):
        return "shape"

    def __str__(self):
        return "{0}, area: {1}, perim: {2}".format(self.nametype(),
                                               self.calculate_area(),
                                               self.calculate_perimeter())


class rectangle(shape):
    def __init__(self, side1, side2):
        self.__side1 = side1
        self.__side2 = side2

    def calculate_area(self):
        return self.__side1 * self.__side2

    def calculate_perimeter(self):
        return (self.__side1 * 2) + (self.__side2 * 2)

    def nametype(self):
        return "rectangle"


class square(rectangle):
    def __init__(self, side):
        self._rectangle__side1 = side
        self._rectangle__side2 = side

    def nametype(self):
        return "square"


class equiTria(shape):
    def __init__(self, side, height):
        self.__side = side

    def calculate_area(self):
        self.__height = self.calculate_perimeter() / (2 * sqrt(3))
        return (self.__side * self.__height)/2

    def calculate_perimeter(self):
        return self.__side * 3

    def nametype(self):
        return "equiTria"


class circle(shape):
    def __init__(self, radius):
        self.__radius = radius

    def calculate_area(self):
        return pi * pow(self.__radius, 2)

    def calculate_perimeter(self):
        return 2 * pi * self.__radius

    def nametype(self):
        return "circle"


class pentagon(shape):
    def __init__(self, side):
        self.__side = side
        self.__apothem = side * 0.688

    def calculate_perimeter(self):
        return self.__side * 5

    def calculate_area(self):
        return (self.calculate_perimeter() * self.__apothem) / 2

    def nametype(self):
        return "pentagon"


class hexagon(shape):
    def __init__(self, side):
        self.__side = side

    def calculate_area(self):
        self.__apothem = self.__side * 0.866
        return (self.calculate_perimeter() * self.__apothem) / 2

    def calculate_perimeter(self):
        return self.__side * 6

    def nametype(self):
        return "hexagon"


def shapeIterator(listOfShapes):
    print("Generator...")
    print(listOfShapes)
    listOfShapessoretedbyArea = shape.sortedByArea(listOfShapes)
    for shapes in listOfShapessoretedbyArea:
        yield str(shapes)

Upvotes: 0

Views: 67

Answers (2)

Martijn Pieters
Martijn Pieters

Reputation: 1121864

You rebind shape in your loops, so it is no longer the class, but one instance.

For example, just above your use of the generator:

for shape in listOfShapessoretedbyPeri2:
    print(str(shape))

The variables in the __main__ section are still globals, so that replaced the class used by the generator.

Your options are:

  1. Use a different name for the loop variable; ashape for example.
  2. Use a different name for the class. The Python style guide recommends using CamelCase for class names, so renaming it to Shape would do nicely here.
  3. Put all the code under the if __name__ == '__main__': block in a function, so that variable names like the loop target become locals.

Personally, I'd implement both 2 and 3; avoiding polluting your global namespace is always a good idea, and so is following the almost universally adopted Python style guide; this helps avoid such mistakes in the future.

In addition, if sortedByArea is meant to be a static method, do at least use the @staticmethod decorator. That way it is still useable as a static method even on instances:

class Shape:
    # ...

    @staticmethod
    def sortedByArea(shapes):
        return sorted(shapes, key=lambda x: x.calculate_area())

    @staticmethod
    def sortedByPerim(shapes):
        return sorted(shapes, key=lambda x: x.calculate_perimeter())

Upvotes: 2

user2357112
user2357112

Reputation: 280564

You reused the shape variable, once for the shape class and once for the loop variable in all your for shape in loops.

Upvotes: 1

Related Questions