views:

82

answers:

4

I have a class that represents a pretty complex object. The objects can be created by many ways: incremental building, by parsing text strings in different formats and by analyzing binary files. So far my strategy was as follows:

  • Have the constructor (__init__, in my case) initialize all the internal variables to None

  • Supply different member functions to populate the object

  • Have those functions return the new, modified object to the caller so we can do sd = SuperDuper().fromString(s)

For example:

class SuperDuper:
    def __init__(self):
        self.var1 = None
        self.var2 = None
        self.varN = None

    ## Generators
    def fromStringFormat1(self, s):
        #parse the string
        return self 
    def fromStringFormat2(self, s):
        #parse the string
        return self
    def fromAnotherLogic(self, *params):
        #parse params
        return self
    ## Modifiers (for incremental work)
    def addThis(self, p):
        pass
    def addThat(self, p):
        pass
    def removeTheOtherOne(self, p):
        pass

The problem is that the class becomes very huge. Unfotunately I am not familiar with OOP pattern designs, but I assume that there is a more ellegant solution for this problem. Is taking the generator functions out of the class (so that fromString(self, s) becomes superDuperFromString(s) a good idea?

+1  A: 

I don't believe it would be, since those all relate directly to the class regardless.

What I would do is make the constructor take arguments to initialize the fields (defaulting to None of course), then turn all the from*() methods into classmethods that construct new objects and return them.

Ignacio Vazquez-Abrams
should the from*() methods also modify the existing object, or just create a new one? If the former is the case, then my code above is exactly what you mean, except for the `__init__` part
bgbg
They will be classmethods, so there will be no existing object to modify.
Ignacio Vazquez-Abrams
Oh, now I got the class part
bgbg
A: 

I don't think it is a bad design to have conversion/creation methods inside the class. You could always move it to a separate class and then you would have a Simple Factory which is a very light-weight design pattern.

I'd keep them in the class though :)

willcodejavaforfood
+2  A: 

What might be a better idea in your case is dependency injection and inversion of control. The idea is to create another class that has all of the settings that you are parsing out of all of these different sources. Then subclasses can define the method to actually parse it. Then when you instantiate the class, pass an instance of the settings class to it:

class Settings(object):
    var1 = None
    var2 = None
    var3 = None

    def configure_superduper(self, superduper):
        superduper.var1 = self.var1
        # etc

class FromString(Settings):
    def __init__(self, string):
        #parse strings and set var1, etc.

class SuperDuper(object):
    def __init__(self, settings): # dependency injection  
        settings.configure_superduper(self)  # inversion of control
        # other initialization stuff

sup = SuperDuper(object, FromString(some_string))

Doing it this way has the advantage of adhering more closely to the single responsibility principle which says that a class should only have one (likely to occur) reason to change. If you change the way you're storing any of these strings, then the class has to change. Here, we're isolating that into one simple, separate class for each source of data.

If on the other hand, you think that the data that's being stored is more likely to change than the way it's stored, you might want to go with class methods as Ignacio is suggesting because this is (slightly) more complicated and doesn't really buy you much in that case because when that happens you have to change two classes in this scheme. Of course it doesn't really hurt much either because you'll only have to change one more assignment.

aaronasterling
A: 

Have those functions return the new, modified object to the caller so we can do sd = SuperDuper().fromString(s)

Rarely is this a good idea. While some Python library classes do this, it's not the best approach.

Generally, you want to do this.

class SuperDuper( object ):
    def __init__(self, var1=None, var2=None, var3=None):
        self.var1 = var1
        self.var2 = var2
        self.varN = var3

    def addThis(self, p):
        pass
    def addThat(self, p):
        pass
    def removeTheOtherOne(self, p):
        pass

class ParseString( object ):
    def __init__( self, someString ):
        pass
    def superDuper( self ): 
        pass

class ParseString_Format1( ParseString ):
    pass

class ParseString_Format2( ParseString ):
    pass

def parse_format1( string ):
    parser= ParseString_Format1( string )
    return parser.superDuper()

def parse_format2( string ):
    parser= ParseString_Format2( string )
    return parser.superDuper()

def fromAnotherLogic( **kw ):
    return SuperDuper( **kw )

There are two unrelated responsibilities: the object and the string representations of the object.

Do Not Conflate Objects and String Representations.

Objects and Parsing must be kept separate. After all, the compiler is not part of the code that's produced. An XML parser and the Document Object Model are generally separate objects.

S.Lott