correct class inheritance

Question

I've written some code that allows the term 'job' to be used universally to perform a unique task. The specific jobs can be chosen through setting an initial variable "job_type". From that initial variable a particulay subclass is chosen to perform the appropriate job. Maybe the code will make more sense :)

if __name__=='__main__':
    # these variables would normally be called in from a config file
    job_type = 'job1'
    uni_var = 10

    job_select = superClass(job_type, uni_var)
    job_select.job()        


class superClass(object):
    def __init__(self, job_type, uni_var):
        self.job_type = job_type   
        self.uni_var = uni_var

        if self.job_type == 'job1':
            self.jobChoice = option1()
        else:
            self.jobChoice = option2()

    # This is the definition called by the main function it then
    # redirects the request to the appropriate job sub class   
    def job(self):
        self.jobChoice.job()

class option1(superClass):
    def __init__(self):
        pass

    def job(self):
        print 'job option 1'
        print uni_var

class option2(superClass):
    def __init__(self):
        pass

    def job(self):
        print 'job option 2'
        print uni_var

The thought behind this code was to allow a single/constant 'main' function, to action a variety of unique tasks based purely on the variable 'job_type'. Which it seems to be doing fine.

My question (as a very inexperienced coder) is, have I gone about this the right way or is there a better way to do things?

Also, have I set up the variable 'uni_var' correctly in the superClass to be correctly shared across all/any superClass subclasses?

Thanks.

Answer 1

You are actually not using inheritance at all in your original code, and your code contains a severe bug, but masks it from appearing.

You create an instance of superClass, which stores job_type and uni_var, and then instantiates either option1 or option2 and stores a reference to that. The "subclasses" are initialised with no data (so they have no job_type or uni_var attributes, and you have to override __init__ to do-nothing methods to avoid error).

When you call job_select.job(...), superClass explicitly delegates to its job_choice attribute. So there's no useful inheritance going on; you've overridden everything about superClass in your subclasses, and instead having job_select be an instance of different sub-classes depending on the job_type and using method resolution to call the right code, job_select is always a superClass which contains an option1 or option2 and explicitly delegates to it.

The serious bug I mentioned: neither option1 nor option2 actually contains any information about the job, so their job methods can't do anything interesting. You call print uni_var, which doesn't work (it would normally be print self.uni_var to get the uni_var of this job), but seems to work because you have a global called uni_var. As soon as you start doing anything more complicated this scheme will fall over badly.

@srgerg's answer is a good example of what you could do to actually use inheritance and factory functions to solve your problem. @CatPlusPlus's answer is a good example of how you can use more appropriate tools for the very simple code in this example (may not be what you need if your real requirements are more complex what you've got written at the moment).

Answer 2

Don't use a class as a factory, it's silly. You just need a consistent interface across implementations.

class JobA(object):
    def do_job(self, arg):
        print 'job a', arg

class JobB(object):
    def do_job(self, arg):
        print 'job b', arg

job_types = {
    'job1': JobA, 'job2': JobB
}

job_type = 'job1'
uni_var  = 10

job = job_types[job_type]()
job.do_job(uni_var)

Hell, if the jobs don't keep state, they shouldn't be classes, either, but functions instead.

def job_a(arg):
   ...

def job_b(arg):
   ...

job = job_types[job_type]
job(uni_var)

Answer 3

I suspect that what you really want is to use the Factory Method Pattern here.

You could change your code to something like this:

if __name__=='__main__':
    # these variables would normally be called in from a config file
    job_type = 'job1'
    uni_var = 10

    job_select = superClass.optionFactory(job_type, uni_var)
    job_select.job()        


class superClass(object):
    def __init__(self, job_type, uni_var):
        self.job_type = job_type   
        self.uni_var = uni_var

    # This is the definition called by the main function it then
    # redirects the request to the appropriate job sub class   
    def job(self):
        raise NotImplementedError()

    @staticmethod
    def optionFactory(job_type, uni_var):
        "Return an instance of superClass based on job_type and uni_var."
        if job_type == "job1":
            return option1(job_type, uni_var)
        else:
            return option2(job_type, uni_var)

class option1(superClass):
    def __init__(self, job_type, uni_var):
        super(option1, self).__init__(job_type, uni_var)

    def job(self):
        print 'job option 1'
        print uni_var

class option2(superClass):
    def __init__(self, job_type, uni_var):
        super(option2, self).__init__(job_type, uni_var)

    def job(self):
        print 'job option 2'
        print uni_var

However, notice that this implementation will require that superClass be changed every time a new subclass is created. Another alternative would be to make the optionFactory method a standalone function (rather than a method of superClass). Like this:

if __name__=='__main__':
    # these variables would normally be called in from a config file
    job_type = 'job1'
    uni_var = 10

    job_select = optionFactory(job_type, uni_var)
    job_select.job()        


class superClass(object):
    def __init__(self, job_type, uni_var):
        self.job_type = job_type   
        self.uni_var = uni_var

    # This is the definition called by the main function it then
    # redirects the request to the appropriate job sub class   
    def job(self):
        raise NotImplementedError()

class option1(superClass):
    def __init__(self, job_type, uni_var):
        super(option1, self).__init__(job_type, uni_var)

    def job(self):
        print 'job option 1'
        print uni_var

class option2(superClass):
    def __init__(self, job_type, uni_var):
        super(option2, self).__init__(job_type, uni_var)

    def job(self):
        print 'job option 2'
        print uni_var

def optionFactory(job_type, uni_var):
    "Return an instance of superClass based on job_type and uni_var."
    if job_type == "job1":
        return option1(job_type, uni_var)
    else:
        return option2(job_type, uni_var)