7

I have some Python classes that, if simplified, look like:

class base: def __init__(self, v): self.value = v def doThings(self): print "Doing things." def doMoreThings(self): print "Doing more things." def combine(self, b): self.value += b.value class foo(base): def showValue(self): print "foo value is %d." % self.value class bar(base): def showValue(self): print "bar value is %d." % self.value

The base class contains methods (represented above by doThings and doMoreThings) which implement functionality common to both the foo and bar subclasses. The foo and bar subclasses differ essentially in how they interpret the value field. (Above, they only differ by what they show when printing it, but in my actual application, they do several other things which are more complicated.) base can be thought of as "abstract": users only ever work with foos and bars. base exists only as a home for code common to its subclasses.

The method I want to ask about is combine, which lets you take two of these objects and make a third. Because foo and bar interpret value differently, it doesn't make sense to combine two subclasses of different types: you can combine two foos to get a foo or two bars to get a bar but not a foo and a bar. Even so, the procedure for combine is the same for all subclasses, so it makes sense to have it factored out and defined in one place.

I would probably like to signal an error if a user tries to combine two incompatible objects, but I don't see a way to do this without introducing ugly typechecks. Is it good practice to do this? Or should I do the usual thing and not check, document the issue, and assume that the user won't try to use combine in a way that wasn't intended, even though such use would appear to "succeed" and return a garbage object instead of raising an error?

Thank you for your help.

Improve this question

4 Answers 4

Reset to default
3

I see several approaches here:

  1. Don't check anything and trust the user to do the right thing. Might be appropriate or dangerous, depending on the situation.

  2. Check the type. You are right that it looks ugly, but is the easiest thing.

  3. Don't have a value, but name them the way they are intended to have (pressure, temperature) and let them combine themselves.

  4. Same as 3, but additionally have the subclasses have a property which maps accesses to .value to their respective "real" value variable. This way, you can keep the .__init__(), but the .combine() will have to be done by every subclass.

  5. Same as 4, but don't use property, but a self-crafted descriptor. In this answer, I show how it could be done.

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

2

Alter the combine method

def combine(self, b): if (not(self.__class__.__name__ == b.__class__.__name__)): raise TypeError("%s cannot combine with %s" % ( self.__class__.__name__, b.__class__.__name__)) else: self.value += b.value

Alternatively, alter the class definition for base to 

class base(object):

and then a simpler, nicer, happier combine method is possible (thanks glglgl)

def combine(self, b): if (not(type(self) == type(b))): raise TypeError("%s cannot combine with %s" % ( type(self), type(b) )) else: self.value += b.value
Improve this answer

4 Comments

Comparing __class__.__name__ is IMHO very ugly. A much better way would be comparing type(self) == type(b) or using isinstance(b, type(self)).
@glglgl type(self) == type(b) is always true as both are instance. isinstance has the same problem. Agree it's ugly but...
Ouch! Didn't see the OP uses old-style classes... then it's clear. (I try to avoid them wherever possible.) But with isinstance it should work woth old-style classes as well (just tested).
@glglgl yes, possibly the best solution would be to declare base an object and then use type(self) == type(b). Editing my answer
0

When I worked on production code that shall not fail, I used a decorator ensuring that a risky function will not explode, turn into a fireball and blow my whole system.

def bulltproof(func): def _bulletproof(*args, **kwargs): try: result = func(*args, **kwargs) except Exception as exc: raise BulletproofError(exc) return result return _bulletproof

Use it like that:

@bulletproof def some_risky_function(): #....

And catch only one error:

try: some_risky_function() except Bulletproof: ...

Instead of raising the error you can return None, log error or turn it off for tests. Of course, this approach is useful when you want to ensure that your program will recover from error, but sometimes it's better to die than continue with error

Improve this answer

Comments

0

Since .combine() is conceptually doing completely different things depending on whether the objects are foo or bar, I think it makes sense NOT to have the function in the abstract parent, but to keep it in the derived classes. Even if it's the same code, combining two instances of foo would be a different operation from combining two instances of bar. If the code for .combine() is actually very lengthy, you could create a private method in the base class to do the work, but only call it from functions defined in the derived class.

Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.