Bad Practice to run code in constructor thats likely to fail?

Question

Hello everyone,

my question is rather a design question. In Python, if code in your "constructor" fails, the object ends up not being defined. Thus:

someInstance = MyClass("test123") #lets say that constructor throws an exception
someInstance.doSomething() # will fail, name someInstance not defined.

I do have a situation though, where a lot of code copying would occur if i remove the error-prone code from my constructor. Basically my constructor fills a few attributes (via IO, where a lot can go wrong) that can be accessed with various getters. If I remove the code from the contructor, i'd have 10 getters with copy paste code something like :

1. is attribute really set?
2. do some IO actions to fill the attribute
3. return the contents of the variable in question

I dislike that, because all my getters would contain a lot of code. Instead of that I perform my IO operations in a central location, the constructor, and fill all my attributes.

Whats a proper way of doing this?

Answer 1

I'm not a Python developer, but in general, it's best to avoid complex/error-prone operations in your constructor. One way around this would be to put a "LoadFromFile" or "Init" method in your class to populate the object from an external source. This load/init method must then be called separately after constructing the object.

Answer 2

One common pattern is two-phase construction, also suggested by Andy White.

First phase: Regular constructor.

Second phase: Operations that can fail.

Integration of the two: Add a factory method to do both phases and make the constructor protected/private to prevent instantation outside the factory method.

Oh, and I'm neither a Python developer.

Answer 3

In C++ at least, there is nothing wrong with putting failure-prone code in the constructor - you simply throw an exception if an error occurs. If the code is needed to properly construct the object, there reallyb is no alternative (although you can abstract the code into subfunctions, or better into the constructors of subobjects). Worst practice is to half-construct the object and then expect the user to call other functions to complete the construction somehow.

Answer 4

If the code to initialise the various values is really extensive enough that copying it is undesirable (which it sounds like it is in your case) I would personally opt for putting the required initialisation into a private method, adding a flag to indicate whether the initialisation has taken place, and making all accessors call the initialisation method if it has not initialised yet.

In threaded scenarios you may have to add extra protection in case initialisation is only allowed to occur once for valid semantics (which may or may not be the case since you are dealing with a file).

Answer 5

Again, I've got little experience with Python, however in C# its better to try and avoid having a constructor that throws an exception. An example of why that springs to mind is if you want to place your constructor at a point where its not possible to surround it with a try {} catch {} block, for example initialisation of a field in a class:

class MyClass
{
    MySecondClass = new MySecondClass();
    // Rest of class
}

If the constructor of MySecondClass throws an exception that you wish to handle inside MyClass then you need to refactor the above - its certainly not the end of the world, but a nice-to-have.

In this case my approach would probably be to move the failure-prone initialisation logic into an initialisation method, and have the getters call that initialisation method before returning any values.

As an optimisation you should have the getter (or the initialisation method) set some sort of "IsInitialised" boolean to true, to indicate that the (potentially costly) initialisation does not need to be done again.

In pseudo-code (C# because I'll just mess up the syntax of Python):

class MyClass
{
    private bool IsInitialised = false;

    private string myString;

    public void Init()
    {
        // Put initialisation code here
        this.IsInitialised = true;
    }

    public string MyString
    {
        get
        {
            if (!this.IsInitialised)
            {
                this.Init();
            }

            return myString;
        }
    }
}

This is of course not thread-safe, but I don't think multithreading is used that commonly in python so this is probably a non-issue for you.

Answer 6

It is not bad practice per se.

But I think you may be after a something different here. In your example the doSomething() method will not be called when the MyClass constructor fails. Try the following code:

class MyClass:
def __init__(self, s):
    print s
    raise Exception("Exception")

def doSomething(self):
    print "doSomething"

try:
    someInstance = MyClass("test123")
    someInstance.doSomething()
except:
    print "except"

It should print:

test123
except

For your software design you could ask the following questions:

• What should the scope of the someInstance variable be? Who are its users? What are their requirements?

• Where and how should the error be handled for the case that one of your 10 values is not available?

• Should all 10 values be cached at construction time or cached one-by-one when they are needed the first time?

• Can the I/O code be refactored into a helper method, so that doing something similiar 10 times does not result in code repetition?

• ...

Answer 7

There is a difference between constructor in C++ and __init__ method in Python. Task of the constructor is to construct object and if it fails no destructor is called. Therefore if any resources where acquired before exception was thrown the cleanup shall be done before exiting constructor. Thus some prefer two-phase construction with most of construction done outside constructor (ugh).

Python have much more clean two-phase construction (construct than initialize) but many people confuse __init__ method (initializer) with constructor. The actual constructor in Python is called __new__. Contrary to C++ it does not take an (half-constructed) instance but returns one. The __init__ task is to initialize the created instance. When exception is raised in __init__ the destructor (__del__) is called as expected because object was already created (not properly initialized but still) when __init__ was called.

Answering your question:

In Python, if code in your "constructor" fails, the object ends up not being defined.

That's not precise. If exception is raised __init__ the object is created but not initialized properly (e.g. some attributes are not assigned). But when it does you probably don't have any reference to this object. Only destructor (if any) needs to check whether attribute actualy exist.

Whats a proper way of doing this?

In Python initialize objects in __init__ and fear not about exceptions. In C++ use RAII.

---

Update [about resource management]:

In garbage collected languages if you are dealing with resources, especially limited ones such as database connections, it's better not to release them in destructor. This is because objects are destroyed in non deterministic way and if you happen to have a loop of references, which is not always easy to tell, and at least one of objects in loop have destructor defined they will never be destroyed. Garbage collected languages have other means of dealing with resources. In Python it's with statement.

Answer 8

seems Neil had a good point: my friend just pointed me to this:

http://en.wikipedia.org/wiki/Resource_Acquisition_Is_Initialization

which is basically what Neil said...

Answer 9

This isn't an answer, it's a question to add to this. Is there a way after INIT is performed and before the object is returned to the user to run some kind of AFTER_INIT code? I have been working with Python for about two weeks now so I am not sure if there is.

For example, I am using a CacheFTPHandler from urllib2. Every call to urllib2 should use this cached handler. But currently each method call that uses urllib2 has to do the following:

    if not self.cached_ftp_handler_created:
        self.create_cached_ftp_handler()
    with closing(urllib2.urlopen(str)) as url:
         blah blah


    def create_cached_ftp_handler(self):
      #called each time a urllib2 is used, but should only create one handler...
      if not self.cached_ftp_handler_created:
        hdlr = urllib2.CacheFTPHandler()
        hdlr.setMaxConns(self.__max_number_of_ftp_connections)
        opener = urllib2.build_opener(hdlr)
        urllib2.install_opener(opener)
        self.cached_ftp_handler_created = True

Rather than constantly checking to see if the handler has been cached, I'd rather do it automatically after the INIT method completes. I can't do it IN the INIT method for the reasons mentioned above.

I could do it with a Factory as mentioned above, but wondered if there might be some other way?

Thanks,

Paul

Answer 10

Thanks, jerryjvl. That's pretty much what I am doing!