Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I described to a colleague why a constructor calling a method can be an antipattern.

example (in my rusty C++)

class C {
public :
    C(int foo);
    void setFoo(int foo);
private:
    int foo;
}

C::C(int foo) {
    setFoo(foo);
}

void C::setFoo(int foo) {
    this->foo = foo
}

I would like to motivate better this fact through your additional contribute. If you have examples, book references, blog pages, or names of principles, they would be very welcome.

Edit: I'm talking in general, but we are coding in python.

share|improve this question
    
Is this a general rule or specific to particular languages? –  ChrisF Feb 17 '11 at 15:43
    
Which language? In C++ it's more than an anti-pattern: parashift.com/c++-faq-lite/strange-inheritance.html#faq-23.5 –  LennyProgrammers Feb 17 '11 at 15:44
    
@Lenny222, the OP talks about "class methods", which - to me at least - means non-instance methods. Which therefore can't be virtual. –  Péter Török Feb 17 '11 at 15:53
2  
@Alb In Java it is perfectly okay. What you shouldn't do though is explicitly pass this to any of the methods you call from the constructor. –  biziclop Feb 17 '11 at 16:02
3  
@Stefano Borini: If you're coding in Python, why not show the example in Python instead of rusty C++? Also, please explain why this is a bad thing. We do it all the time. –  S.Lott Feb 17 '11 at 16:22
show 5 more comments

8 Answers

up vote 17 down vote accepted

You have not specified a language.

In C++ a constructor must beware when calling a virtual function, in that the actual function it is calling is the class implementation. If it is a pure virtual method without an implementation, this will be an access violation.

A constructor may call non-virtual functions.

If your language is Java where functions are generally virtual by default it makes sense that you have to be extra careful.

C# seems to handle the situation the way you would expect: you can call virtual methods in constructors and it calls the most final version. So in C# not an anti-pattern.

A common reason for calling methods from constructors is that you have multiple constructors that want to call a common "init" method.

Note that destructors will have the same issue with virtual methods, thus you cannot have a virtual "cleanup" method that sits outside of your destructor and expect it to get called by the base-class destructor.

Java and C# don't have destructors, they have finalizers. I don't know the behaviour with Java.

C# appears to handle clean-up correctly with this regard.

(Note that although Java and C# have garbage collection, that only manages memory allocation. There is other clean-up that your destructor needs to do that is not releasing memory).

share|improve this answer
10  
There are a few small errors here. Methods in C# are not virtual by default. C# has different semantics than C++ when calling a virtual method in a constructor; the virtual method on the most derived type will be called, not the virtual method on the portion of the type currently being constructed. C# does call its finalization methods "destructors" but you are right that they have the semantics of finalizers. Virtual methods called in C# destructors work the same way as they do in constructors; the most derived method is called. –  Eric Lippert Feb 17 '11 at 15:55
    
@Péter : I intended instance methods. sorry for the confusion. –  Stefano Borini Feb 17 '11 at 15:59
1  
@Eric Lippert. Thank you for your expertise on C#, I have edited my answer accordingly. I am unknowledgeable on that language, I know C++ very well and Java less well. –  CashCow Feb 17 '11 at 17:02
3  
You're welcome. Note that calling a virtual method in a base class constructor in C# is still a pretty bad idea. –  Eric Lippert Feb 17 '11 at 17:08
add comment

OK, now that the confusion regarding class methods vs instance methods is cleared up, I can give an answer :-)

The problem is not with calling instance methods in general from a constructor; it is with calling virtual methods (directly or indirectly). And the main reason is that while inside the constructor, the object is not yet fully constructed. And especially its subclass parts are not at all constructed while the base class constructor is executing. So its internal state is inconsistent in a language dependent way, and this may cause different subtle bugs in different languages.

C++ and C# have already been discussed by others. In Java, the virtual method of the most derived type will be called, however that type is not yet initialized. So if that method is using any fields from the derived type, those fields may not yet be initialized properly at that point in time. This problem is discussed in detail in Effecive Java 2nd Edition, Item 17: Design and document for inheritance or else prohibit it.

Note that this is a special case of the general problem of publishing object references prematurely. Instance methods have an implicit this parameter, but passing this explicitly to a method can cause similar problems. Especially in concurrent programs where if the object reference is published prematurely to another thread, that thread can already call methods on it before the constructor in the first thread finishes.

share|improve this answer
    
(+1) "while inside the constructor, the object is not yet fully constructed." Same as "class methods vs instance". Some programming languagues consider it constructed when entering the contructor, as if the programmer where assign values to constructor. –  umlcat Mar 19 '11 at 18:35
add comment

I wouldn't consider method calls here to be an antipattern in itself, more a code smell. If a class supplies a reset method, that returns an object to its original state, then calling reset() in the constructor is DRY. (I'm not making any statements about reset methods).

Here's an article that might help satisfy your appeal for authority: http://misko.hevery.com/code-reviewers-guide/flaw-constructor-does-real-work/

It isn't really about calling methods, but about constructors that do too much. IMHO, calling methods in a constructor is a smell that might indicate that a constructor is too heavy.

This is related to how easy it is to test your code. Reasons include:

  1. Unit testing involves lots of creation and destruction - therefore construction should be fast.

  2. Depending on what those methods do, it may make it difficult to test discrete units of code without relying on some (potentially untestable) precondition set up in the constructor (e.g. get info from a network).

share|improve this answer
add comment

It's not a general-purpose issue. It's a problem in C++, specifically when using inheritance and virtual methods, because object construction happens backwards, and the vtable pointer(s) get reset with each constructor layer in the inheritance hierarchy, so if you're calling a virtual method you might not end up getting the one that actually corresponds to the class you're trying to create, which defeats the whole purpose of using virtual methods.

In languages with sane OOP support, that set the vtable pointer correctly from the start, this problem doesn't exist.

share|improve this answer
add comment

There are two issues with calling a method:

  • calling a virtual method, which can either do something unexpected (C++) or use parts of the objects that haven't been initialized yet
  • callling a public method (which should enforce the class invariants), since the object is not necessarily complete yet (and thus its invariant may not hold)

There is nothing wrong with calling a helper function, as long as it does not fall in the two previous cases.

share|improve this answer
add comment

Philosophically, the purpose of constructor is to turn a raw chunk of memory into an instance. While the constructor is being executed, the object does not yet exist, therefore calling its methods is a bad idea. You may don't know what they do internally after all, and they may rightfully consider the object to at least exist (duh!) when they are called.

Technically, there may be nothing wrong with that, in C++ and especially in Python, it's up to you to be careful.

Practically, you should limit calls to only such methods that initialize class members.

share|improve this answer
add comment

I don't buy this. In an object-oriented system, calling a method is pretty much the only thing you can do. In fact, that's more or less the definition of "object-oriented". So, if a constructor can't call any methods, then what can it do?

share|improve this answer
    
Initialize the object. –  Stefano Borini Feb 17 '11 at 22:23
    
@Stefano Borini: How? In an object-oriented system, the only thing you can do is call methods. Or to look at it from the opposite angle: anything is done by calling methods. And "anything" obviously includes object initialization. So, if, in order to initialize the object, you need to call methods, but constructors can't call methods, then how can a constructor initialize the object? –  Jörg W Mittag Feb 17 '11 at 22:27
    
it's absolutely not true that the only thing you can do is to call methods. You may just initialize the state without any call, directly to the internals of your object... The point of the constructor is to make an object in a consistent state. If you call other methods, these may have troubles handling an object in a partial state, unless they are methods specifically made to be called from the constructor (typically as helper methods) –  Stefano Borini Feb 17 '11 at 22:30
    
@Stefano Borini: "You may just initialize the state without any call, directly to the internals of your object." Sadly, when that involves a method, what do you do? Copy and past the code? –  S.Lott Feb 18 '11 at 0:55
1  
@S.Lott: no, I call it, but I try to keep it a module function instead of an object method, and have it provide return data I can put into the object state in the constructor. If I really have to have an object method, I will make it private and clarify that it's for initialization, such as giving it a proper name. I would never, however, call a public method to set object status from the constructor. –  Stefano Borini Feb 18 '11 at 10:22
show 3 more comments

In O.O.P. theory, it shouldn't matter, but in practice, each O.O.P. programming language handles constructors different. I don't use static methods very often.

In C++ & Delphi, If I had to given initial values to some properties ("field members"), and the code is very extended, I add some secondary methods as extension of the constructors.

And don't called other methods that do more complex stuff.

As for properties "getters" & "setters" methods, I usually use private / protected variables to store their state, plus "getters" & "setters" methods.

In the constructor I assign "default" values to the properties state fields, WITHOUT calling the "accessors".

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.