18

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?

lispmachine
  • 4,407
  • 1
  • 22
  • 31
Tom
  • 3,115
  • 6
  • 33
  • 38
  • ok, I'll remember that, I'm still new here and want to give credit to the people who spend their valuable time, trying to answer my questions, I really appreciate it! – Tom Jun 02 '09 at 08:44
  • This question come far from Python and even from explaining what's all about RAII or why do we can/cannot do operations in constructor which may raise exception? – lispmachine Jun 02 '09 at 11:04
  • 1
    @Neil Butterworth : this is not really important since you can accept another better anwser later. – Bite code Oct 24 '09 at 18:04

8 Answers8

38

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

Python has a much cleaner two-phase construction (construct, then initialize). However, many people confuse an __init__ method (initializer) with a constructor. The actual constructor in Python is called __new__. Unlike in C++, it does not take an instance, but returns one. The task of __init__ is to initialize the created instance. If an exception is raised in __init__, the destructor __del__ (if any) will be called as expected, because the object was already created (even though it was not properly initialized) by the time __init__ was called.

Answering your question:

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

That's not precisely true. If __init__ raises an exception, the object is created but not initialized properly (e.g., some attributes are not assigned). But at the time that it's raised, you probably don't have any references to this object, so the fact that the attributes are not assigned doesn't matter. Only the destructor (if any) needs to check whether the attributes actually exist.

Whats a proper way of doing this?

In Python, initialize objects in __init__ and don't worry 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 the destructor. This is because objects are destroyed in a 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 the objects in the loop has a destructor defined, they will never be destroyed. Garbage collected languages have other means of dealing with resources. In Python, it's a with statement.

Alan
  • 1,889
  • 2
  • 18
  • 30
lispmachine
  • 4,407
  • 1
  • 22
  • 31
  • 1
    Ahhh, at last, a anwser from somebody who actually codes in Python. Thks, I was going to say that. What's more, it's perfectly OK to surround your object create with a try catch... – Bite code Oct 24 '09 at 18:07
  • `with statement`... You guys mention this aaaaall the time. Consider a program with GUI. You create object in dialog's `__init__` and want to release it on dialog close. This object holds some non-memory resource. What now?... – cubuspl42 Nov 14 '13 at 09:22
  • @cubuspl42 In case of event-based program I would go for explicit releasing. For example in GTK there is "destroy" signal to listen for https://developer.gnome.org/pygtk/stable/class-gtkobject.html#signal-gtkobject--destroy – lispmachine Nov 15 '13 at 09:54
  • The behaviour here is not magic, but a consequence of `__init__` raising an exception and hence the caller does not get a reference to the new object. Hence, a caveat for this answer is that if your `__init__` has established a circular reference to `self`, `__del__` will not be called immediately on failure, but when that circular reference is cleaned up later. So you can't break that circular reference from `__del__`, as it won't be called until it's already broken the reference. – TBBle Apr 30 '21 at 06:17
20

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.

  • Objective-C uses this 2-phase construction pattern pretty much everywhere for creating objects, so it must have some merit. – Andy White Jun 02 '09 at 08:16
  • 5
    Throwing an exception in C++ constructor does not call the destructor (since the object is not yet constructed). You'll have to make sure you clean up whatever you did manage to construct before throwing. auto_ptrs are handy. – laalto Jun 02 '09 at 08:16
  • @Andy I can't speak for Objective C, but this approach generally means storing flags of some sort to indicate construction status - this is error prone and (in C++ at least) unecessary. –  Jun 02 '09 at 08:18
  • @laalto As you say, use of aotu_ptr and (better still) non-pointer sub-objects solves this problem. The alternative approach has exactly the same problems, except you now need to know when NOT to destroy a sub object. –  Jun 02 '09 at 08:19
  • 1
    Yeah, I agree that it's ugly. I like the idea of hiding this in a factory, maybe that makes it a little better... – Andy White Jun 02 '09 at 08:20
  • Python has a nice&handy garbage collector just like java, so letting the object go out of scope or having no references to it anymore wont make explicit destruction necessary. I'm just not certain which style is better practise, throwing exceptions out of my constructor, or using a two phase approach. Certainly the code would look nicer with exceptions, but probably it violates OOP principles ? I heard this many times, to avoid failure-prone code in the constructor. And at least speaking for me, im a creature of habit, so maybe its better to not rely on garbage collection, if i ever move to c – Tom Jun 02 '09 at 08:23
  • 'Tom When people say "I heard ..." one would like to know where they heard it. There is absolutely nothing wrong with throwing exceptions from a constructor. C++ does not have garbage collection, so this is not a GC issue. And if you ever move to C, your whole programming style will have to change anyway. –  Jun 02 '09 at 08:29
  • Yes thats true Neil. "I heard" was related to many different locations, as usually i dont give much on single statements. I just kept hearing this on forums, computer science lectures, many different places. My whole style will change indeed, but its better to get used to some common practises, because some things wont change if you have a solid understand of what you are doing. I have been coding for quite a while now, but some patterns are pretty much language independant, and the two phase thing sounds like a solid pattern to build on and to remember, thanks to all of you :) – Tom Jun 02 '09 at 08:33
  • hm now I'm getting into a dilemma of not knowing whats the "correct" solution (if you can call it that, as there's always more than one). – Tom Jun 02 '09 at 08:41
  • One way of deciding is to see how much code you have to write to implement the two different approaches - choose the one that requires the least code. –  Jun 02 '09 at 08:45
  • Why is this answer overrated? Two-phase construction might be ugly in C++ unless using factories but not freeing resources by half-constructing object is wrong. RAII is both correct and readable. – lispmachine Jun 02 '09 at 10:06
4

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?

  • ...

Ralph
  • 5,154
  • 1
  • 21
  • 19
3

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.

Andy White
  • 86,444
  • 48
  • 176
  • 211
  • 1
    Alright, i guess you are right. I hoped there would be a nicer approad, because I guess then I have to go through all of these checks. guess my code will look like that then: 1. empty constructor 2. some fileLoading method 3. every getter making a call whether the file has been loaded before doing its job
    ugly, but I guess thats the only possible way
    – Tom Jun 02 '09 at 08:15
  • 3
    The factory idea suggested by laalto might be a good idea too. The factory will hide the 2-phased construction, so your app code doesn't need to know about it. (I'd upvote his answer, but I'm out of votes today :). I still stand by my statement that it's better to not throw exceptions from a constructor. – Andy White Jun 02 '09 at 08:19
3

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.

laalto
  • 150,114
  • 66
  • 286
  • 303
0

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).

jerryjvl
  • 19,723
  • 7
  • 40
  • 55
0

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.

Justin
  • 84,773
  • 49
  • 224
  • 367
  • I personally dislike having each and every single method in a class have a copy pasted initialized check. It's error prone if someone else works on the code and forgets it, and in my eyes it is also not pretty, but maybe that is only a subjective matter of taste. I really hoped to be able to avoid something like that :/ – Tom Aug 29 '11 at 19:26
0

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...

Tom
  • 3,115
  • 6
  • 33
  • 38
  • 1
    But it's C++/Ada not Python related. Also in managed (garbage collected) languages resource acquisition/release is not performed by constructors/destructors. – lispmachine Jun 02 '09 at 11:01