1

I have written this snippet of code to illustrate the problem. Below the code itself you can see the console printout.

In my program, Polygon class objects store vertex coordinates in a list of vectors pointing to each vertex. All the Translate() function has to do is iterate over each vector in the list and add the argument vector to each item. Simple, right?

The Vector class has its own overloaded __add__ function.

When I wrote and tested the code, I found out that members of that list change only while I'm iterating. As soon as it's done, all coordinates revert to their original values.

After I've discovered this problem, on a hunch I made another function - Manual_Translate(), which calculates vector components by hand (without calling Vector.__add__)

class Vector():

    def __init__(self, X, Y):
        self.x = X
        self.y = Y

    def __add__(self, Other_Vector):

        return Vector((self.x + Other_Vector.x),(self.y + Other_Vector.y))

class Polygon():

    def __init__(self, point_list):

        self.Points = []

        for item in point_list:
            self.Points.append (Vector(item[0], item[1]))

    def Translate (self, Translation):

        for point in self.Points:
            point += Translation
            print (point.x, point.y)  #printout from the same loop

    def Manual_Translate (self, Translation):

        for point in self.Points:
            point.x += Translation.x
            point.y += Translation.y
            print (point.x, point.y)

    def Report (self):

        for point in self.Points:
            print (point.x, point.y)  #printout from another loop

vertices = [[0,0],[0,100],[100,100],[100,0]]

Square = Polygon (vertices)
print ("A: we used __add__ function")
Square.Translate (Vector(115,139))
print ("")
Square.Report()
print ("\nB: we calculated vector sum by hand")
Square.Manual_Translate (Vector(115,139))
print ("")
Square.Report()

The result: If I use __add__, the value changes get lost. If I add vectors by hand - they stay. What am I missing? Does __add__ actually have anything to do with this problem, or not? What's going on?

A: we used __add__ function
115 139
115 239
215 239
215 139

0 0
0 100
100 100
100 0

B: we calculated vector sum by hand
115 139
115 239
215 239
215 139

115 139
115 239
215 239
215 139
norien
  • 113
  • 3

3 Answers3

5

your problem is because your __add__ function is returning a new instance of a Polygon rather than actually changing the values of your instance.

return Vector((self.x + Other_Vector.x),(self.y + Other_Vector.y))

should instead be:

self.x += Other_Vector.x
self.y += Other_Vector.y
return self

As Pynchia pointed out, this only works because you are assuming that a += operator is being called. To better suit for all class implementations, explicitly define both the __add__ and __iadd__ data model methods:

  • __add__ for things like c = b + a should return a new instance and so your original code would have been fine.

  • __iadd__ is for in-place operator += and should then return itself like my code has.

R Nar
  • 5,465
  • 1
  • 16
  • 32
  • Of course! Thanks! I'm rather surprised this issue hasn't bitten me in the ass earlier. – norien Dec 07 '15 at 17:14
  • this answer may suit the specific need (i.e. incrementing the object `a += b`), but would fail/not behave as expected in other situations (e.g. `c = a + b`) – Pynchia Dec 07 '15 at 17:39
  • 1
    @Pynchia, very good and quite an important thing to point out. Editted to included `__iadd__` – R Nar Dec 07 '15 at 17:45
  • @Pynchia, @R Nar, I see. Will define both. – norien Dec 07 '15 at 17:48
1

in

    def Translate (self, Translation):

        for point in self.Points:
            point += Translation
            print (point.x, point.y)  #printout from the same loop

after the increment, the name point is assigned a new object, i.e. it references a new object, which is the result of the sum of the previous object it referred to (i.e. the element in the list) and the other object (Translation), as __add__ returns a new instance.

Therefore the original list element is never touched.

Whereas, in

    def Manual_Translate (self, Translation):

        for point in self.Points:
            point.x += Translation.x
            point.y += Translation.y
            print (point.x, point.y)

point refers to the list element and if you alter its attributes you mutate the original object.

Try doing

    def Translate (self, Translation):

        for point in self.Points:
            print(id(point))
            point += Translation
            print(id(point))
            print (point.x, point.y)  #printout from the same loop

One solution has been given above by @R Nar, having __add__ update the attributes directly, but it modifies the original object and it may not suit other types of expressions:

e.g.

point_c = point_a + point_b

results in point_a being modified and point_c referring to the same object (i.e. point_a)

Example

>>> class A:
...  def __init__(self,n):
...   self.n = n
...  def __add__(self,other):
...   self.n = self.n + other.n
...   return self
... 
>>> a=A(2)
>>> b=A(3)
>>> c=a+b
>>> a
<__main__.A object at 0x7ff1b9bece10>
>>> a.n
5
>>> c
<__main__.A object at 0x7ff1b9bece10>
>>> c.n
5

So, the best thing to do is to act on both __add__ and __iadd__ (see @R Nar's edit)

Pynchia
  • 10,996
  • 5
  • 34
  • 43
0

You issue is that your overloaded Add function doesn't actually change the values of self.x and self.y. It returns a new vector that represents those additions. However, that new vector isn't being stored in the list Points, it is just held in the variable point.

Update your overload addition so that it updates the values of self.x and self.y rather than return a new vector.

RD3
  • 1,089
  • 10
  • 24