views:

43

answers:

3

Python refactoring

Both the add and sub are very similar. How does one re-factor code like this? The logic is basically inverse of each other.

class point(object):

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

      def add( self, p ):
          x = self.x + p.x
          y = self.y + p.y
          return point( x, y )

      def sub( self, p ):
          x = self.x - p.x
          y = self.y - p.y
          return point( x, y )
A: 

Here's something you could do.

def __add__(self, p):   # used this so that you can add using the + operator
    x = self.x + p.x
    y = self.y + p.y
    return point(x, y)

def __sub__(self, p):
    return self + point(-p.x, -p.y)
JAB
+3  A: 

First, standard practice is to capitalize classes (so Point, not point). I'd make use of the __add__ and __sub__ (and possibly __iadd__ and __isub__) methods, as well. A first cut might look like this:

class Point(object):
    def __init__(self, x, y):
        self.x = x
        self.y = y

    def __add__(self, p):
        return Point(self.x + p.x, self.y + p.y)

    def __sub__(self, p):
        return Point(self.x - p.x, self.y - p.y)

I know you're looking to pull the logic out into a single method, something like:

class Point(object):
    def __init__(self, x, y):
        self.x = x
        self.y = y

    def _adjust(self, x, y):
        return Point(self.x + x, self.y + y)

    def __add__(self, p):
        return self._adjust(p.x, p.y)

    def __sub__(self, p):
        return self._adjust(-p.x, -p.y)

... but that seems more complicated, without much gain.

Chris B.
This is what I was looking for. I didn't know that python had the __add__ and __sub__. Also the underscore in front of _adjust, what is the meaning and convention for that? I agree that adding an adjust method is more complicated without gain. Thanks for this response.
foxhop
The underscore in `_adjust` is simply a Python convention that the method is "semi-private". There's virtually no difference, except that it signals to others reading the code they should probably avoid calling it directly unless they know what they're doing. Note: this is _not_ the case for a double-underscore (like `__adjust`). See http://docs.python.org/tutorial/classes.html#private-variables for details.
Chris B.
Thanks for the help!
foxhop
+1  A: 

What about that:

import operator

class point(object):
    def __init__( self, x, y ):
        self.x, self.y = x, y

    def _do_op(self, op, p):
        x = op(self.x, p.x)
        y = op(self.y, p.y)
        return point(x, y)

    def add( self, p ):
        return self._do_op(operator.add, p)

    def sub( self, p ):
        return self._do_op(operator.sub, p)
Johannes Weiß
This was what I came up with. While it doesn't really gain you much in this simple example, it DOES allow you to internally hide the representation of the point. The methods don't care what variables are involved in adding or subtracting, just that it should be done to the point.
stw_dev