1
\$\begingroup\$

I'm using linear algebra as my basis. One very specific change I'd like to make is to allow for equality tests between points and vectors with an unequal number of components by assuming that all undefined components are zero (all points and vectors have infinite components; equality tests stop testing once a check for both components return 'undefined'). Mostly I'd like to do this just because it seems kind of cool to do, so it is not remarkably important.

But in essence, I really just want someone to rip this code apart and tell me why I'm doing everything wrong (or tell me that I'm doing things pretty alright). The goal is just to have a nice, sane Point and Vector system that is very flexible and can be understood by anyone--and used by anyone--for any purpose they'd like. Everything I'm building will rely on the following code, so I want to make sure that my design patterns are reasonable and flexible.

import math, sys, os

TOLERANCE = 0.00001

class SizeError(Exception):
    def __init__(self, *args):
        self.args = args
        print "Size Error between " + str(args)

class P(object):
    def __init__(self, *args):
        if len(args) == 1:
            try:
                self.components = tuple(args[0])
                self.size = len(args[0])
            except TypeError:
                self.components = tuple(args)
                self.size = len(args)
        else:
            self.components = args
            self.size = len(args)
    def __getitem__(self, key):
        return self.components[key]
    def __repr__(self):
        result = ""
        for i in range(0, self.size):
            result += str(self[i]) + ", "
        return "<Point (" + str(self.size) + ") : "  + result[:-2] + ">"
    def __eq__(self, other):
        if self.size != other.size:
            raise SizeError(self, other)
        for i in range(0, self.size):
            result = self[i] - other[i]
            if math.fabs(result) > TOLERANCE:
                return False
        return True
    def __add__(self, other):
        if other.size != self.size:
            raise SizeError(self, other)
        result = []
        for i in range(0, self.size):
            result.append(self[i] + other[i])
        return P(result)
    def __sub__(self, other):
        if other.size != self.size:
            raise SizeError(self, other)
        result = []
        for i in range(0, self.size):
            result.append(self[i] - other[i])
        return V(result)

class V(P):
    def __repr__(self):
        result = ""
        for i in range(0, self.size):
            result += str(self[i]) + ", "
        return "<Vector (" + str(self.size) + ") : "  + result[:-2] + ">"
    def __add__(self, other):
        if other.size != self.size:
            raise SizeError(self, other)
        result = []
        for i in range(0, self.size):
            result.append(self[i] + other[i])
        return V(result)
    def __mul__(self, scalar):
        result = []
        for i in range(0, self.size):
            result.append(self[i] * scalar)
        return V(result)
    def __neg__(self):
        return self * -1
    def dotProduct(self, other):
        result = 0
        if self.size != other.size:
            raise SizeError(self, other)
        for i in range(0, self.size):
            result += self[i] * other[i]
        return result
    def normalize(self):
        result = []
        length = self.getLength()
        if math.fabs(length) < TOLERANCE: return self.getZero()
        for i in range(0, self.size):
            result.append(self[i] / length)
        return V(result)
    def getLength(self):
        result = 0
        for i in range(0, self.size):
            result += self[i]**2
        return math.sqrt(result)
    def getZero(self):
        return self * 0
    def isZero(self):
        zero = self.getZero()
        return self == zero
\$\endgroup\$
3
  • \$\begingroup\$ Why don't you use numpy? It's fast, reliable and extremely powerful. \$\endgroup\$
    – P3trus
    Commented Nov 9, 2012 at 20:29
  • \$\begingroup\$ To be honest, I expect that that's precisely what I'll eventually end up doing. But I want to make sure I understand good architecture before I start relying on other people's excellent architecture. \$\endgroup\$
    – user19115
    Commented Nov 9, 2012 at 20:46
  • \$\begingroup\$ You got good feedback. My 2 cents - I don't see how vector 'is-a' point - the hierarchy doesn't make sense. \$\endgroup\$
    – avip
    Commented Nov 10, 2012 at 7:24

1 Answer 1

2
\$\begingroup\$
import math, sys, os

TOLERANCE = 0.00001

class SizeError(Exception):
    def __init__(self, *args):
        self.args = args
        print "Size Error between " + str(args)

You shouldn't print in an exception constructor. At most, you should define a __repr__ for printing purposes. But you can probably get away with not doing that and trusting the default.

class P(object):

The name doesn't give me much clue as to what this class is.

    def __init__(self, *args):
        if len(args) == 1:
            try:
                self.components = tuple(args[0])
                self.size = len(args[0])
            except TypeError:
                self.components = tuple(args)
                self.size = len(args)
        else:
            self.components = args
            self.size = len(args)

Instead of assigning size three times, use self.size = len(self.components) at the end. In fact, consider not storing the size at all and just looking at the size of the components. I'd also suggest not accepting three different ways of passing in the parameters. Pick one and make it be used everywhere.

    def __getitem__(self, key):
        return self.components[key]
    def __repr__(self):
        result = ""
        for i in range(0, self.size):
            result += str(self[i]) + ", "

Add strings is expensive. But python has a really nice join function. So you could do:

result = ",".join(map(str,self))

And it'll actually take care of everything else for you

        return "<Point (" + str(self.size) + ") : "  + result[:-2] + ">"
    def __eq__(self, other):
        if self.size != other.size:
            raise SizeError(self, other)
        for i in range(0, self.size):

You don't need the zero.

            result = self[i] - other[i]
            if math.fabs(result) > TOLERANCE:

I'd combine those two lines

                return False
        return True
    def __add__(self, other):
        if other.size != self.size:
            raise SizeError(self, other)
        result = []
        for i in range(0, self.size):
            result.append(self[i] + other[i])

Use zip, and it'll be a bit faster to use a list comprehension:

result = [x+y for x, y in zip(self, other)]

        return P(result)
    def __sub__(self, other):
        if other.size != self.size:
            raise SizeError(self, other)
        result = []
        for i in range(0, self.size):
            result.append(self[i] - other[i])
        return V(result)

class V(P):

I wouldn't have vector inherit from point. I'd have them both inherit from some other more generic class. A vector is not a point, coding like it is one could be confusing.

    def dotProduct(self, other):

Python convention is to have names which are lowercase_with_underscores for methods.

        result = 0
        if self.size != other.size:
            raise SizeError(self, other)
        for i in range(0, self.size):
            result += self[i] * other[i]
        return result

Here I'd use

return sum(x * y for x, y in zip(self, other))


    def normalize(self):
        result = []
        length = self.getLength()
        if math.fabs(length) < TOLERANCE: return self.getZero()
        for i in range(0, self.size):
            result.append(self[i] / length)
        return V(result)
    def getLength(self):
        result = 0
        for i in range(0, self.size):
            result += self[i]**2
        return math.sqrt(result)

    def getZero(self):
        return self * 0
    def isZero(self):
        zero = self.getZero()
        return self == zero

Both of these will be inefficent implementations. They should work, but they'll be relatively slow. The fact being any Point/Vector class implemented in python will be unforgivably slow. That's why we typically implement such things in extensions like numpy.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.