• 08/26/13 08:08 AM
    Sign in to follow this  
    Followers 0

    Wade Not In Unknown Waters: Part One

    General and Gameplay Programming

    Code_Analysis

    We decided to write several small posts on how C/C++ programmers play with fire without knowing it. The first post will be devoted to an attempt to explicitly call a constructor.

    Programmers are lazy creatures. That's why they tend to solve a task using minimal amounts of code. This aim is praiseworthy and good. But the main point is to not get too involved in the process and stop at the right time.

    For example, programmers are too lazy to create a single initialization function in a class so that it could be called from various constructors later. They think: "What do I need an extra function for? I'd rather call one constructor from the other". Unfortunately, sometimes programmers can't solve even such a simple task. It is to detect such unsuccessful attempts that I'm implementing a new rule in PVS-Studio. Here is, for instance, a code sample I have found in the eMule project:

    class CSlideBarGroup { public: CSlideBarGroup(CString strName, INT iIconIndex, CListBoxST* pListBox); CSlideBarGroup(CSlideBarGroup& Group); ... } CSlideBarGroup::CSlideBarGroup(CSlideBarGroup& Group) { CSlideBarGroup( Group.GetName(), Group.GetIconIndex(), Group.GetListBox()); }

    Let's examine more attentively how the last constructor is implemented. The programmer decided that the code

    CSlideBarGroup( Group.GetName(), Group.GetIconIndex(), Group.GetListBox());

    simply calls the other constructor. Nothing of the kind. A new unnamed object of the CslideBarGroup type is created and destroyed right afterwards.

    It appears that the programmer has actually called the other constructor. But he/she has not quite done the same thing he/she intended: the class fields remain uninitialized.

    Such errors are just half the trouble. Some people do know how to call the other constructor really. And they do it. I wish they didn't know :)

    For instance, the above given code could be rewritten in this way:

    CSlideBarGroup::CSlideBarGroup(CSlideBarGroup& Group) { this->CSlideBarGroup::CSlideBarGroup( Group.GetName(), Group.GetIconIndex(), Group.GetListBox()); }

    or in this way:

    CSlideBarGroup::CSlideBarGroup(CSlideBarGroup& Group) { new (this) CSlideBarGroup( Group.GetName(), Group.GetIconIndex(), Group.GetListBox()); }

    Now one data initialization constructor is really calling the other constructor.

    If you see a programmer doing this, deal him/her one flick on his/her forehead for yourself and one more flick on my behalf. The cited examples contain very dangerous code and you should understand well how they work! Being written for the purpose of petty optimization (programmers are too lazy to write a separate function), this code might do more harm than good. Let's see more closely why such constructs sometimes work but most often don't.

    class SomeClass { int x,y; public: SomeClass() { new (this) SomeClass(0,0); } SomeClass(int xx, int yy) : x(xx), y(yy) {} };

    This code will work correctly. It is safe and works well, since the class contains primary data types and is not a descendant of other classes. In this case, a double constructor call is harmless.

    Let's consider another example where an explicit constructor call causes an error (the sample is taken from this discussion on the StackOverflow website):

    class Base { public: char *ptr; std::vector vect; Base() { ptr = new char[1000]; } ~Base() { delete [] ptr; } }; class Derived : Base { Derived(Foo foo) { } Derived(Bar bar) { new (this) Derived(bar.foo); } }

    When we call the new (this) Derived(bar.foo); constructor, the Base object is already created and fields initialized. The repeated constructor call will cause a double initialization. A pointer to the newly allocated memory area will be written into ptr. As a result, we get a memory leak. The result of double initialization of an object of the std::vector type cannot be predicted at all. But one thing is obvious: such code is inadmissible.

    Conclusion

    An explicit constructor call is needed only in very rare cases. In common programming practice, an explicit constructor call usually appears due to a programmer's wish to reduce the code's size. Don't do that! Create an ordinary initialization function.

    This is how the correct code should look:

    class CSlideBarGroup { void Init(CString strName, INT iIconIndex, CListBoxST* pListBox); public: CSlideBarGroup(CString strName, INT iIconIndex, CListBoxST* pListBox) { Init(strName, iIconIndex, pListBox); } CSlideBarGroup(CSlideBarGroup& Group) { Init(Group.GetName(), Group.GetIconIndex(), Group.GetListBox()); } ... };

    P.S. Explicit call of one constructor from the other in C++11 (delegation)

    The new C++11 standard allows you to perform the call of constructors from other constructors (known as delegation). It enables you to create constructors that use the behavior of other constructors without added code. This is an example of correct code:

    class MyClass { std::string m_s; public: MyClass(std::string s) : m_s(s) {} MyClass() : MyClass("default") {} };
    0


    Sign in to follow this  
    Followers 0


    User Feedback

    Create an account or sign in to leave a review

    You need to be a member in order to leave a review

    Create an account

    Sign up for a new account in our community. It's easy!


    Register a new account

    Sign in

    Already have an account? Sign in here.


    Sign In Now

    Fairywingsrpg

    • 5
      
    0

    Share this review


    Link to review
    mousetail

    • 5
      
    0

    Share this review


    Link to review
    HappyCoder

    • 5
      
    0

    Share this review


    Link to review
    jms bc

    • 5
      
    0

    Share this review


    Link to review
    Mito

    • 5
      
    0

    Share this review


    Link to review
    Bacterius

    • 5
      
    0

    Share this review


    Link to review
    DeafTV

    • 5
      
    0

    Share this review


    Link to review
    TheChubu

    • 5
      
    0

    Share this review


    Link to review
    ongamex92

    • 5
      
    0

    Share this review


    Link to review
    EMascheG

    • 5
      
    0

    Share this review


    Link to review
    leeor_net

    • 5
      
    0

    Share this review


    Link to review
    JonBMN

    • 5
      
    0

    Share this review


    Link to review
    Ron AF Greve

      
    0

    Share this review


    Link to review
    Six222

      
    0

    Share this review


    Link to review
    Ectara

    • 5
      
    0

    Share this review


    Link to review
    Kaptein

    • 5
      
    0

    Share this review


    Link to review
    Zaoshi Kaba

    • 5
      
    0

    Share this review


    Link to review
    Greenhouse

    • 5
      
    0

    Share this review


    Link to review