3
\$\begingroup\$

EDIT: I have made another go at this using metaclasses here. I think it's a much better approach.

I have the following classes. validator is a decorator that receives a class which defines validation criteria for a decorated function. ValidateKeys is the validation criteria for this example (other criteria I plan to include later are maximum, minimum, and rounding/decimal places). Node is a parent class with a decorated setter, and Node2D and Node3D are child classes differing only in the validation criteria each uses.

I'm wondering if this is a good solution, if there are problems with doing it this way (i.e., inheritance) that I'm not seeing, etc etc. Any criticism is welcome.

One thing that I don't like, and doesn't seem to make sense, is that I had to move the list of valid keys from the validator to the parent class itself (see old code version below). This doesn't really make sense to me because I'd like to separate the valid list from the implementation of the Node class [the Node class is just and example; I will transform it into something more general, for making Node (valid members: x,y), Element (valid members: i,j,k,l), Boundary (valid member: b) etc etc].

class validator(object):
    def __init__(self, TheValidator, *args, **kwargs):
        print("New validator Object")
        self.TheValidator = TheValidator(*args,**kwargs)
    def __call__(self,f):
        def wrapped_f(instance, *args,**kwargs):
            self.TheValidator(instance, *args, **kwargs)
            return f(instance,*args,**kwargs)
        return wrapped_f

class ValidateKeys(object):
    def __init__(self,*keysIterable):
        print("New ValidateKeys Object")
    def __call__(self, instance, **kwargs):
        for a in kwargs:
            if not a in instance.valid_keys:
                raise Exception()
        instance.__dict__.update(kwargs)

class Node(object):
    def __new__(klass, *valid_keys):
        Node.valid_keys = valid_keys
        return super(Node, klass).__new__(klass)
    @property
    def coords(self):
        return self.__dict__
    @coords.setter
    def coords(self,Coords):
        self.set_coords(**Coords)
    @validator(ValidateKeys)
    def set_coords(self,**Coords):
        pass

class Node3D(Node):
    def __new__(klass):
        return super(Node3D, klass).__new__(klass, 'x','y','z')

class Node2D(Node):
    def __new__(klass):
        return super(Node2D, klass).__new__(klass, 'x','y')

In case it helps make clear what I'm trying to accomplish, an older version of my solution is below prior to creating the Node parent class. I added the parent class to cut down on duplicate code, but to do that I had to refactor a lot, including moving valid_keys from the ValidateKeys class to the Node class.

class validator(object):
    def __init__(self, TheValidator, *args, **kwargs):
        self.TheValidator = TheValidator(*args,**kwargs)
    def __call__(self,f):
        def wrapped_f(instance, *args,**kwargs):
            self.TheValidator(instance, *args, **kwargs)
            return f(instance,*args,**kwargs)
        return wrapped_f

class ValidateKeys(object):
    def __init__(self,*keysIterable):
        self.valid_keys = keysIterable
    def __call__(self, instance, **kwargs):
        for a in kwargs:
            if not a in self.valid_keys:
                raise Exception()
        instance.__dict__.update(kwargs)

class Node3D(object):
    @property
    def coords(self):
        return self.__dict__
    @coords.setter
    def coords(self,Coords):
        self.set_coords(**Coords)
    @Validator(ValidateKeys, 'x','y','z')
    def set_coords(self,**Coords):
        pass

class Node2D(object):
    @property
    def coords(self):
        return self.__dict__
    @coords.setter
    def coords(self,Coords):
        self.set_coords(**Coords)
    @Validator(ValidateKeys, 'x','y')
    def set_coords(self,**Coords):
        pass
\$\endgroup\$
  • 1
    \$\begingroup\$ I would recommend another parameter and call the class Node less specific and you don't write all that code twice \$\endgroup\$ – Malachi Oct 30 '14 at 15:49
  • \$\begingroup\$ I think that I know less Python than you do. but here is what I would do, I would create a method around the @Validator and overload it (not sure how you do that in Python) to take in 2 or 3 coordinates, or you could also send it a variable telling it how many coordinates it's going to receive and use an if statement. \$\endgroup\$ – Malachi Oct 30 '14 at 16:01
  • \$\begingroup\$ I suggest discarding this approach completely and making both classes using collections.namedtuple() instead. In fact, your use case is almost identical to the example in Python's documentation. There is a key difference, which is that tuples are immutable. I see immutability as being a benefit for coordinates. (If you have, say, a Polygon2D class, you don't want code to be able to alter the vertices by mutating their coordinates.) \$\endgroup\$ – 200_success Oct 30 '14 at 19:22
  • \$\begingroup\$ Using a named tuple was my first thought, too! I actually DO want to be able to alter my vertices. That is the entire point of the module: to load a list of nodes with x's and y's from several external file formats, add and delete nodes as necessary, but also EDIT the x's and y's (move the nodes around), and export again. Being able to edit the x's and y's is why I went with a dict instead. But maybe I should try doing this another way. \$\endgroup\$ – Rick supports Monica Oct 30 '14 at 19:36
  • \$\begingroup\$ Maybe a Point should be a named tuple, and then when "moving" a Node, what I'm really doing under the hood is deleting the old Point and replacing it with a new Point. That way Points can be immutable. \$\endgroup\$ – Rick supports Monica Oct 30 '14 at 19:40
1
\$\begingroup\$

One thing that I don't like, and doesn't seem to make sense, is that I had to move the list of valid keys from the validator to the parent class itself

That's not only something to dislike -- it is actually broken. If you do this...

n3 = Node3D()
n2 = Node2D()
n3.coords = {'x':1,'y':2,'z':3}

...you will get Exception because the second line has caused Node.valid_keys to be overwritten. To fix that, you could eliminate all three __new__ methods and instead define the subclasses like this:

class Node3D(Node):
    valid_keys ='x','y','z'

class Node2D(Node):
    valid_keys ='x','y'

However, the whole approach is not very good because one can still do for example this:

>>> node = Node2D()
>>> node.coords = {'y': 2, 'x': 1}
>>> node.z = 3
>>> node.coords
{'y': 2, 'x': 1, 'z': 3} 

Using a namedtuple as discussed in comments is a better idea.

\$\endgroup\$
  • \$\begingroup\$ Gosh. You're so right. It's horrible. \$\endgroup\$ – Rick supports Monica Oct 31 '14 at 12:27
  • \$\begingroup\$ I have been convinced that using named tuples to represent nodes is a great idea. This way, instead of keeping track of node numbers, I can simply include the nodes inside of elements and boundaries directly, e.g., Node = namedtuple('Node','x y'), Beam = namedtuple('Beam','i j'), n1=Node(0,0),n2=Node(1,1), b1=Beam(n1,n2). And since Node is hashable I can have a dict or OrderedDict of nodes to attach additional (mutable) information. But there's a a few problems. First, if I do n1._replace, b1 is not updated. Also, in my data two different Nodes can be located in the same place. \$\endgroup\$ – Rick supports Monica Oct 31 '14 at 13:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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