57

Am I doing this right? I'm trying to delegate a C++ class constructor as it's basically the same code repeating 3 times.. I read up on C++x11 and read that g++ 4.7.2 allows this but I'm not sure if I'm doing it right:

Bitmap::Bitmap(HBITMAP Bmp)
{
   //Construct some bitmap stuff..
}

Bitmap::Bitmap(WORD ResourceID)
{
   HBITMAP BMP = (HBITMAP)LoadImage(GetModuleHandle(NULL), MAKEINTRESOURCE(ResourceID), IMAGE_BITMAP, 0, 0, LR_SHARED);

   Bitmap(BMP);   //Delegates to the above constructor? Or does this create a temporary?
}

OR do I need to do:

Bitmap::Bitmap(HBITMAP Bmp)
{
   //Construct some bitmap stuff..
}

Bitmap::Bitmap(WORD ResourceID) : Bitmap((HBITMAP)LoadImage(GetModuleHandle(NULL), MAKEINTRESOURCE(ResourceID), IMAGE_BITMAP, 0, 0, LR_SHARED))
{
}
Brandon
  • 22,723
  • 11
  • 93
  • 186
  • 5
    The second one is right. Does it not compile? – chris Dec 19 '12 at 20:58
  • 2
    Both compile. I was just wondering if I needed the initialization list version or if the first is acceptable. – Brandon Dec 19 '12 at 20:59
  • 15
    Ah, I see your dilemma. The first creates an unnamed object and does nothing with it, much like `int(5);` would. – chris Dec 19 '12 at 21:00
  • 2
    Actually, the first example shouldn't compile since `Bitmap(BMP);` declares a local variable, named `BMP` of type `Bitmap`. It does not create a temporary, unnamed object. It should result in a multiple definition error since `BMP` already exists (type `HBITMAP`). Furthermore, `Bitmap` is required to have a empty/default/standard constructor for this line to be compileable. – Pixelchemist Jun 12 '14 at 00:13
  • 2
    @Pixelchemist What are you talking about.. `Bitmap(BMP);` definitely declares a temporary object of type `Bitmap` with `BMP` as a parameter. This code was actually compiled at the time of the post. I was curious and asked what it does. – Brandon Jun 12 '14 at 07:45
  • @Brandon Pixelchemist was right, this is an instance of the [most-vexing parse](https://stackoverflow.com/questions/1424510/my-attempt-at-value-initialization-is-interpreted-as-a-function-declaration-and). Just because it *vexed* you, doesn't mean it's not a declaration [as by the explicit rules stated in the C++ standard](https://timsong-cpp.github.io/cppwp/n4659/stmt.ambig#1). If MSVC accepted the code (without at least emitting a diagnosis), it was a conformance bug. Newer MSVC versions reject the code (with a somewhat cryptic error message). – Arne Vogel Sep 26 '18 at 12:15

4 Answers4

49

You need to do the second. Delegating constructors only works in the constructor's initialization list, otherwise you'll just create a temporary or do other mistakes like you mentioned.

Pubby
  • 51,882
  • 13
  • 139
  • 180
  • 1
    Ah this is exactly what I needed to know. If you can, can you answer this: Do I need to delete the HBitmap in the second one using DeleteObject? Or is it fine to just leave it as is? Also I'm going to mark your answer as accepted in 10 mins because it won't let me accept before that. – Brandon Dec 19 '12 at 21:03
  • 2
    Yes, of course you need to call `DeleteObject`. That's irrelevant to the rest of the code in your question, though. If you call `LoadImage`, and it succeeds, then you need to free the result at some point. Seems like the place to do that is in the class's destructor. – Rob Kennedy Dec 19 '12 at 21:17
47

The correct syntax is 

struct Foo {
  Foo(char x, int y) : _x{x}, _y(y) {}
  Foo(int y) : Foo('a', y) {}

  char _x;
  int _y;
};

Your first example creates a temporary that is destroyed right away.

Gui Lima
  • 166
  • 1
  • 9
log0
  • 10,489
  • 4
  • 28
  • 62
2

If you want to use constructor delegation after some imperative logic, you can opt to move-assign *this from a temporary:

Foo() {
  // calculate stuff…
  *this = Foo(stuff, calculated, above);
}
ThatsJustCheesy
  • 1,370
  • 14
  • 24
1

The second example using the initializer list is the correct one . First example is going to end up creating a temporary object.

rahul
  • 79
  • 4