16

I create a class whose objects are initialized with a bunch of XML code. The class has the ability to extract various parameters out of that XML and to cache them inside the object state variables. The potential amount of these parameters is large and most probably, the user will not need most of them. That is why I have decided to perform a "lazy" initialization.

In the following test case such a parameter is title. When the user tries to access it for the first time, the getter function parses the XML, properly initializes the state variable and return its value:

class MyClass(object):     
    def __init__(self, xml=None):
        self.xml  = xml
        self.title = None

    def get_title(self):
        if self.__title is None:
            self.__title = self.__title_from_xml()
        return self.__title

    def set_title(self, value):
        self.__title = value

    title = property(get_title, set_title, None, "Citation title")

    def __title_from_xml(self):
        #parse the XML and return the title
        return title         

This looks nice and works fine for me. However, I am disturbed a little bit by the fact that the getter function is actually a "setter" one in the sense that it has a very significant side effect on the object. Is this a legitimate concern? If so, how should I address it?

Boris Gorelik
  • 29,945
  • 39
  • 128
  • 170
  • 3
    Regardless of what the actual answer to the question is, you shouldn't use leading double underscores. They start name mangling, i.e. lots of potential pain and zero gain. Just use a single leading underscore. –  Jan 19 '11 at 19:19
  • 1
    I don't see why it's an issue. – PrettyPrincessKitty FS Jan 19 '11 at 19:22
  • 1
    Minor refactoring proposal: don't initialise `self._title` in the constructor, and replace the condition in the getter by `not hasattr(self, "_title")`. – Sven Marnach Jan 19 '11 at 19:23
  • @delnan: this is how Eclipse's PyDev creates properties by default. – Boris Gorelik Jan 19 '11 at 19:27
  • @Sven: thank you for the suggestion. Do you care to elaborate on why is this a better idea? A separate answer will be ideal, so I can upvote it. – Boris Gorelik Jan 19 '11 at 19:28
  • 2
    @Sven Marnach: `not hasattr(self, '_title')` is imho rather uncommon for lazy initialization, and will also be slower than a normal attribute lookup and a test against `None`. –  Jan 19 '11 at 19:30

3 Answers3

15

This design pattern is called Lazy initialization and it has legitimate use.

Axarydax
  • 16,353
  • 21
  • 92
  • 151
  • cf my comment on the accepted answer - lazy initialization is fine, but that doesn't mean a property access should be allowed to raise anything. If your class is using lazy initialization, either make sure this will never ever imply any exception, or do not lure the user into thinking he's doing a plain safe attribute access and make your getter an explicit method, documenting the fact it may raise this or that exception. – bruno desthuilliers Nov 24 '17 at 09:10
4

While the getter certainly performs a side-effect, that's not traditionally what one would consider a bad side-effect. Since the getter always returns the same thing (barring any intervening changes in state), it has no user-visible side-effects. This is a typical use for properties, so there's nothing to be concerned about.

Gabe
  • 84,912
  • 12
  • 139
  • 238
  • 1
    I beg to disagree: in this case, parsing the xml can raise exceptions, and no one expects an attribute access to raise some xml parsing exception. I had a very similar case in a project I took over and rewrote this part of the code to have the parsing happening at instanciation so there's no exception happening at a later stage if the xml is broken in any way. A get property should NOT raise any exception ever. Do you expect a plain attribute access to raise anything (assuming the attribute exists of course) ? A computed attribute should be as safe as a plain one. – bruno desthuilliers Nov 24 '17 at 09:07
2

Quite some years later but well: while lazy initialization is fine in itself, I would definitly not postpone xml parsing etc until someone accesses the object's title. Computed attributes are supposed to behave like plain attributes, and a plain attribute access will never raise (assuming the attribute exists of course).

FWIW I had a very similar case in some project I took over, with xml parsing errors happening at the most unexpected places, due to the previous developper using properties the very same way as in the OP example, and had to fix it by putting the parsing and validation part at instanciation time.

So, use properties for lazy initialization only if and when you know the first access will never ever raise. Actually, never use a property for anything that might raise (at least when getting - setting is a different situation). Else, dont use a property, make the getter an explicit method and clearly document it might raise this or that.

NB : using a property to cache something is not the problem here, this by itself is fine.

bruno desthuilliers
  • 75,974
  • 6
  • 88
  • 118