2

I'm not sure my approach is good design and I'm hoping I can get a tip. I'm thinking somewhere along the lines of an abstract method, but in this case I want the method to be optional. This is how I'm doing it now...

from pymel.core import *

class A(object):
    def __init__(self, *args, **kwargs):
        if callable(self.createDrivers):
            self._drivers = self.createDrivers(*args, **kwargs)
            select(self._drivers)

class B(A):
    def createDrivers(self, *args, **kwargs):
        c1 = circle(sweep=270)[0]
        c2 = circle(sweep=180)[0]
        return c1, c2

b = B()

In the above example, I'm just creating 2 circle arcs in PyMEL for Maya, but I fully intend on creating more subclasses that may or may not have a createDrivers method at all! So I want it to be optional and I'm wondering if my approach is—well, if my approach could be improved?

Reto Aebersold
  • 16,306
  • 5
  • 55
  • 74
jedmao
  • 10,224
  • 11
  • 59
  • 65
  • Sorry but it's not enough information to comment on design. From the example I can't see any reason to use A and why B is A. As form now I think you are over-thinking this ;) – Paweł Prażak Jan 16 '11 at 09:28
  • 1
    [Don't `import *`](http://docs.python.org/howto/doanddont.html#from-module-import). –  Jan 16 '11 at 09:28
  • 1
    PEP 8 -- Style Guide for Python Code: http://www.python.org/dev/peps/pep-0008/ – Lennart Regebro Jan 16 '11 at 09:50
  • Thanks @Pawel Prażak, but I'm just trying to condense my code into something people will actually read. I definitely have a reason for doing this. My `A` class is actually `RigControl` and the subclasses would be like `CircleControl` or `CrossControl`, etc. I have many shapes I'll be making that I want to inherit all of `RigControl`'s functionality. – jedmao Jan 16 '11 at 11:45
  • @delnan, moving forward I will definitely make this a priority. Thanks for the tip. – jedmao Jan 16 '11 at 11:47
  • @sfjedi so as far as I understand you use objects to manipulate different parts of 3D scene or something like that? As for now I would go for @singularity solution, but to comment on design there's some kind of proof of concept needed with some docs. Also `createDrivers` in `B` uses some arbitrary hardcoded parameters, it doesn't look right. The whole code reminds me of Java ;P – Paweł Prażak Jan 16 '11 at 15:13
  • @Paweł Prażak: Ugh! I hate Java. Yes. I am working with a 3D scene and creating control shapes to control different parts of a 3D character. The process is called character setup and rigging. Notice my `from pymel.core import *`. This allows me to use the `circle` and `select` methods. The `circle` parameters that you see are kwargs that define how much of a circle arc to draw. Did I address your curiosity/concern? – jedmao Jan 16 '11 at 22:22
  • Interesting. Given complicated nature of 3D animation itself I'd keep the code as simple as possible :) – Paweł Prażak Jan 27 '11 at 12:28

3 Answers3

3

You still have a problem, when you will inherit your class B, and this will call A.__init__ and if you don't implement createDrivers in the subclass this line callable(self.createDrivers) will throw an error as that createDrivers doesn't exist (AttributeError) i think if i were you i will do it like so:

class A(object):
    def __init__(self, *args, **kwargs):
       try:
           self._drivers = self.createDrivers(*args, **kwargs)
           select(self._drivers)
       except NotImplementedError:
           pass

    def createDrivers(self, *args, **kwargs):
        raise NotImplementedError("This class wasn't implemented")

class B(A):
    def createDrivers(self, *args, **kwargs):
        c1 = circle(sweep=270)[0]
        c2 = circle(sweep=180)[0]
        return c1, c2

class C(A):
    pass

Another way is to replace callable(self.createDrivers) by hasattr(self, 'createDrivers').

mouad
  • 67,571
  • 18
  • 114
  • 106
  • I can see how this topic would be quite controversial. Everyone is giving examples that work, but none of them are giving me warm fuzzies. I can't decide which approach is best. They all have their pros and cons. This one's con is that it has the *appearance* of being an abstract method when, in fact, it's intent is optional. – jedmao Jan 16 '11 at 11:26
  • 1
    @sfjedi: this method have as an advantage a failing loudly propriety which the others method don't have and i think this is very important when implementing an API or ohter, because let's say someone will create an of C `c = C()` like in my example when it call `c.createDrivers` this will fail as that createDrivers is __not Implemented__ which is not like __AttributeError__ because this error tel the use what is wrong , in the other hand the others method don't do that if someone call `c.createDrivers` nothing happen ! just a silent error the method just exist and don't do nothing !!?? – mouad Jan 16 '11 at 15:46
  • 1
    @Lennart Regebro: the OP question is __Should I be using abstract methods in this Python scenario?__, and for the method being optional, actually i don't understand what you mean and from your answer i don't see even much what is the difference between our two method except that mine has the ability to fail loudly which has a meaning rather in yours you just don't do nothing , like telling someone that he want to call `createDrivers` from a class that didn't implemented it : "well you called the method but nothing happen read the code source or the docstring to know why !!!" – mouad Jan 16 '11 at 16:04
  • @singularity: You have a very good argument. In fact, I feel really bad about doing this because I already accepted @Lennart Regebro's solution, but I'm inclined to accept your arguments. What I'm looking for here is a virtual function that may or may not be implemented and your solution is readable both in the base class and any subclasses. I guess this is what I'm going for. – jedmao Jan 16 '11 at 20:59
  • @Lennart Regebro: This solution actually is optional in that class C executes fine without any errors. I believe this is the preferred and most readable way to solve this scenario. Sorry for all the back and forth, but I was really torn on this one. – jedmao Jan 16 '11 at 21:02
  • @singularity: Since he wants it t be optional, he should not use an abstract method, **because they are not optional**. Your implementation is in principle an abstract method. It will, as abstract methods will, fail loudly. What sfjedi asked for was one that was optional. That's what my solution gives him. Maybe that's not what he *wanted*, but that's a different question. :-) – Lennart Regebro Jan 16 '11 at 21:34
  • @sfjedi: That's not really "optional" in any reasonable sense. :) But if this solution is what you wanted, then the answer to "should I use an abstract method" is "Yes". – Lennart Regebro Jan 16 '11 at 21:35
  • 1
    @Lennart Regebro: i still don't see how my example use "abstract method" because an abstract method should __fail loudly__ when i create a __class__ that inherit from a base class and doesn't implement my "abstract method", not fail when i call the method of an instance (which is what i did) and if i were to use an abstract method the right way to do it is to use the `abc.abstractmethod` decorator, and each class that inherit from the base class __must__ implement the method, i agree that this was the old way to declare an abstract method in python but not anymore now we have the ABC module :) – mouad Jan 16 '11 at 21:48
  • @Lennart Regebro: I totally agree with @singularity here. It's not an abstract method because you can create a subclass of it without getting a `TypeError: Can't instantiate abstract class C with abstract methods createDrivers`. The answer to whether I should use an abstract method is "No" because I don't want the subclasses to fail if they don't override the method, which makes the method optional. I only accept answers if they are truly the answer for the question I asked, regardless of what I meant or wanted. – jedmao Jan 16 '11 at 22:40
  • Except your confusion on creating/instantiating classes you are right, I misremembered your implementation and didn't reread it properly. Your solution is in fact functionally equivalent to mine, but with more lines of code. I was also confused by the comment: "this method have as an advantage a failing loudly" -- No it doesn't... It won't fail loudly. You ignore the not implemented error. I though it *did* fail loudly, and thus was different, but it isn't different and won't fail loudly. – Lennart Regebro Jan 16 '11 at 23:12
2

I would do this:

class A(object):
    def __init__(self, *args, **kwargs):
        self.createDrivers(*args, **kwargs)

    def createDrivers(self, *args, **kwargs):
        "Override"
        pass

class B(A):
    def createDrivers(self, *args, **kwargs):
        self._drivers = blabla
XORcist
  • 4,288
  • 24
  • 32
  • I really like the readability of your solution here, but in fairness to @Lennart Regebro above, his solution was pretty much the same thing and he posted first, so I'm afraid the solution is going to him; though, I'll vote you up. – jedmao Jan 16 '11 at 11:34
  • I don't mean to bicker about some stackoverflow points but I think I posted about one minute before Lennart ;). Thanks for the upvote nonetheless. – XORcist Jan 16 '11 at 12:03
  • It's not a big deal. In hindsight Lennart's answer is more explicit, assigning the output of createDrivers to self._drivers in the __init__ method. I would probably choose his answer because of that. – XORcist Jan 16 '11 at 20:49
  • @möter: BTW, you missed an asterisk on your first **kwargs. – jedmao Jan 16 '11 at 21:03
1

If you want createDrivers to be optional but still always there, the best is not an abstract method, but do implement it in the base class as a noop.

class A(object):
    def __init__(self, *args, **kwargs):
        self._drivers = self.createDrivers(*args, **kwargs)
        select(self._drivers)

    def createDrivers(self, *args, **kwargs):
        """This should be overridden by subclasses if they need custom drivers"""
        pass
Lennart Regebro
  • 167,292
  • 41
  • 224
  • 251
  • but then the `if callable` line would always evaluate as true, no? – jedmao Jan 16 '11 at 10:24
  • Yes, but you can remove that if and add a `if self._drivers:` or similar after the call to `self.createDrivers`. – TryPyPy Jan 16 '11 at 10:34
  • I see where you're going with this and it's a great working suggestion. Tell you what—update your post like you're saying and add the `"Override"` comment like @möter did down below and I'll accept your solution. – jedmao Jan 16 '11 at 11:32
  • @sfjedi: Right, I forgot to remove it. Now gone. – Lennart Regebro Jan 16 '11 at 15:17