Sign in to follow this  
Norman Barrows

Constructor gotcha's?

Recommended Posts

Recently i've been thinking about what form an oo-ish implementation of my typical game architecture would look like. so far, the mapping from procedural to oo code has been amazingly lucid, with some interesting organizational insights gained along the way such as camera.cansee(location) where location includes a bbox or bsphere radius, or AABrect dimensions. in the procedural code there are inits (and some uninits) for many things such as: program, game, generic_graphics_engine, generic_audio_engine, asset pools, etc. naturally, these are prime candidates for custom constructors and destructors. 

 

i'm aware that statically declared objects are initialized in an order not under your direct control. other than that, are there any "gotcha!"s to look out for?  IE issues - things you don't want to do for some reason in a custom constructor or destructor?

 

 

 

 

Share this post


Link to post
Share on other sites

i'm aware that statically declared objects are initialized in an order not under your direct control. other than that, are there any "gotcha!"s to look out for?  IE issues - things you don't want to do for some reason in a custom constructor or destructor?

 

I've had several of annoyances with static variables. Not just initialization order, but also weird issues when trying to use them across library boundaries (if you want to seperate your engine into a library).

Also, it's not just the initialization order of your classes, but also the initialization order of your classes relative to the APIs you might be using. As an example, accidentally loading assets prior to the OpenGL context being created, or trying to output an error message prior to your logger class being initialized. dry.png

 

To enforce an initialization order, you can put them all in a struct - for me, my 'GameStructure' class holds the asset pools and graphics engine and etc... 'GameStructure' being bare-bones owner (composition-wise) of everything except the globals (and I've been removing more and more of the globals, so there's only two or three left).

To enforce a specific initialization time, I explicitly construct the GameStructure after control is actually handed to my program.

Edited by Servant of the Lord

Share this post


Link to post
Share on other sites

I hate that you can't take a pointer to constructors and destructors. Also a lot of the time there's a huge separation between allocation and initialization, though constructors attempt to package this into a single piece of code. Lastly C++ sucks when you define a constructor and yet want to create an array of your object on the stack and you get the error saying that's not possible without a trivial default constructor.

 

It seems like the moment you start really using constructors and destructors you have to subscribe to a bunch of other OO nonsense. At least, this is my own opinion.

 

Edit: Whoops lots of downvotes. I'll try to be more clear next time! I agree with all the corrections everyone made below. It's not that I don't understand how C++ works or why it is the way it is, I just disagree with it. Please take a look at the below posts responding to this one.

Edited by Randy Gaul

Share this post


Link to post
Share on other sites


To enforce a specific initialization time, I explicitly construct the GameStructure after control is actually handed to my program.

 

that leads to a related constructor question i've had with regard to this "oo port".

 

i only need one entity list at the program level that i can re-init with each run of the game, not one i create with each game instance i run.

 

IE in non OO engish:  i have an entity list. i only need to declare it once at program start then init it each time i run a new game or load a saved game. i don't need to new it for each game then dispose it when the game ends. this would indicate the need for an explicit init method, rater than using the constructor. is this how its usually done? 

Share this post


Link to post
Share on other sites


the order of initialization in constructor initializer lists is the *member declaration order*, not the order in which members are lexically arranged in the initialization list.

 

so if i have member variables:

int a;

int b;

 

and in my constructor i say:

b=5;

a=b*2;

 

it will process a=b*2 first?

 

don't recall if i heard of that one before or not. then again, i first learned c++ syntax when it first came out, and haven't really used it much since. so maybe its just yet another thing that i've already forgotten.

 

aside: in college, my roomies had a calender with quotes for the day, one of which was "I've already forgotten what you'll never even know." pretty harsh huh? <g>

 


Avoid overly long parameter lists (prefer passing in structures or pointer/references to structures.

 

yet another point of question in my "OO-port".   all i need to do is create one entity list, one generic_graphics_engine, one camera, etc at program start (typically). so the program object would own them, then pass them to a game object. and in general, things that used to be global would be declared at a higher level, and passed as parameters to lower level code. this could mean a lot of parameters. so i was thinking about using pointers to parameter structs the way d3d does. is this considered the best way to pass around such "globals" ?

Share this post


Link to post
Share on other sites

IE in non OO engish:  i have an entity list. i only need to declare it once at program start then init it each time i run a new game or load a saved game. i don't need to new it for each game then dispose it when the game ends. this would indicate the need for an explicit init method, rater than using the constructor. is this how its usually done?

 
Construction should always create a "valid" object, but it doesn't always have to be in a ready-to-use state. Take for example, std::fstream.
 
You can construct the file stream like this:

std::fstream myFile("file.dat");

This creates a valid and ready-to-use object (assuming you do proper error-checking to make sure the file was actually opened).
 
However, you can also construct it like this:

std::fstream myFile;
 
//Later:
myFile.open("file.dat");

 
The object is constructed in a valid state, but not really ready-to-use (there's probably a better term for this). Later on, it is made ready to use by calling open() with a filepath.
 
For an array of entities, this can be as simple as an "isActive" boolean, with your "init" function more descriptively named.
You could use the assignment operator and "named constructors" (just static member funcs - they aren't real constructors).

entities[getNextFreeIndex()] = Entity::MakeGoblin(...goblin parameters...);
entities[getNextFreeIndex()] = Entity::MakeBuffalo(...buffalo parameters...);

So the default constructor just sets 'isActive' to false.

Edited by Servant of the Lord

Share this post


Link to post
Share on other sites

However, you can also construct it like this:

std::fstream myFile;
 
//Later:
myFile.open("file.dat");
 
The object is constructed in a valid state, but not really ready-to-use (there's probably a better term for this).
Hogwash. It is absolutely ready to use.

So many developers spout details about initialization thinking in terms of today's world, rather than thinking in terms of when the advice came into being.

All guidelines come with context. This context is often forgotten.

Back in the dark ages, (before about 25 years ago) you needed to do a little dance when you made things.

First, you requested a chunk of memory. The memory was not zeroed out as it is today since that is considered a security flaw. The memory contained completely unknown contents.

Second, you needed to ensure the chunk was adequate for your needs, the allocation succeeded, and possibly handle offset issues.

Third, you had to set the memory from an unknown value into some known state. Commonly you would either set the block to zero, or copy in a prebuilt instance, or fill in entries item by item.


In languages like c++ all of this is handled in the constructor. You can set values to a known state, even if that is all zeros. You can also set it to a more complex state of different values.

Constructors can do additional work, but it is usually bad form. In the fstream example the normal usage is to create a pool of streams which are initialized to empty such that the operation is extremely quick and cannot fail, and then do additional operations like opening, reading, and closing as separate tasks knowing these are slow and can potentially fail.

Allowing constructors to do slow tasks or doing tasks that can fail is a big risk that can cause serious problems. If I want an array of 50 file handles I really want them to be empty. I do not want every call to go out to the OS and out to the disk to verify that the names exist, that they can be marked for reading or writing, and then prebuffred. That is a cause of slow constructors.

As a real world example many beginners build constructors for things like models that require a file, then the constructor proceeds to open the model, parse it, open the associated textures, parse them, transfer everything to the graphics card, and finally return with a constructed object. Now if you want to provide that as an optional convenience function that is an option (not one I recommend, but whatever you need). Instead you should provide a default constructor that does nothing more than set all the values to a known state rather than whatever happened to be in memory before you. That is likely all values of zero and null.

Zeros and nulls are a well-defined state, the object is empty and ready to use.

Share this post


Link to post
Share on other sites

Poor phrasing on my part; what you said in your post is not contrary (I think) to what I was trying to convey in mine.

Your "absolutely ready to use" is what I meant by being "in a valid state".

By "ready to use", I meant "ready to read data from the file". It's not "ready to read data from a file", because it hasn't been told what file to read from.

 

The object is allocated.

It is constructed/initialized (so it is "valid" and usable to some extent, but not fully usable).

But there is an intentional delayed "setup" before it can actually be "used" for the purpose it is intended for. (I don't know the terms for this)

Edited by Servant of the Lord

Share this post


Link to post
Share on other sites

are there any "gotcha!"s to look out for?

 

One detail I've bumped recently, is that object allocation is "indeterminately sequenced with respect to the evaluation of the constructor arguments in a new-expression".

I've tried clang++ instead of g++, and perfectly working code crashed.

Problem was exactly about that rule.

I was passing arguments to constructor, derived from memory pool's state (Lua VM stack, actually).

Object has its own allocator, changing that state upon object's creation. So exact ordering was critical, but I didn't noticed that because gcc evaluated arguments first, while clang decided to allocate object first, and only after that evaluate constructor arguments.

#include <stdio.h>
#include <stdlib.h>

int arg_func()
{
    printf("Args calculated\n");
    return rand();
}

struct A
{
    A(int val)
    {
        printf("A::A(int)\n");
    }

    static void* operator new(size_t size)
    {
        printf("A::operator new\n");
        return malloc(size);
    }

    static void operator delete(void* ptr)
    {
        free(ptr);
    }
};

int main()
{
    delete new A(arg_func());
    return 0;
}

If you compile it with g++ you will get output:

Args calculated
A::operator new
A::A(int)

But with clang++ you'll get:

A::operator new
Args calculated
A::A(int)

Probably you will never notice that if you stick with MSVS only. But it's still "gotcha" case.

Share this post


Link to post
Share on other sites
With regard to passing lots of arguments around due to having all non-global state, I remember having a fear it would be lots before I adopted this.

I've been surprised ever since how few things I need to pass around since this approach makes one think far more carefully about what is needed where.

To replicate the mess of a heavily globals-based project with parameter passing would indeed be a nightmare. But one of the chief advantages of avoiding global state is it encourages you to use a principle of least access, which is a boon for any large project.

Share this post


Link to post
Share on other sites

are there any "gotcha!"s to look out for?

 
One detail I've bumped recently, is that object allocation is "indeterminately sequenced with respect to the evaluation of the constructor arguments in a new-expression".
I've tried clang++ instead of g++, and perfectly working code crashed.
Problem was exactly about that rule.
I was passing arguments to constructor, derived from memory pool's state (Lua VM stack, actually).
Object has its own allocator, changing that state upon object's creation. So exact ordering was critical, but I didn't noticed that because gcc evaluated arguments first, while clang decided to allocate object first, and only after that evaluate constructor arguments.
#include <stdio.h>
#include <stdlib.h>

int arg_func()
{
    printf("Args calculated\n");
    return rand();
}

struct A
{
    A(int val)
    {
        printf("A::A(int)\n");
    }

    static void* operator new(size_t size)
    {
        printf("A::operator new\n");
        return malloc(size);
    }

    static void operator delete(void* ptr)
    {
        free(ptr);
    }
};

int main()
{
    delete new A(arg_func());
    return 0;
}
If you compile it with g++ you will get output:
Args calculated
A::operator new
A::A(int)
But with clang++ you'll get:
A::operator new
Args calculated
A::A(int)
Probably you will never notice that if you stick with MSVS only. But it's still "gotcha" case.


The order of evaluation of function arguments has always been unspecified. This isn't a gotcha so much as just depending on unspecified behavior.

Share this post


Link to post
Share on other sites

IMO the most important aspect of OOP? Invariants!

When you are declaring a class you are introducing a new type.

 

The constuctor is responsible to ensure that the object is initialized with a valid state. Every public method should make sure to leave your object in a valid state satisfying the class invariant.

Once you have internalized this concept, your code will become much more robust.

Share this post


Link to post
Share on other sites


The order of evaluation of function arguments has always been unspecified

But that didn't look like passing multiple arguments to same function and expecting they would be calculated in some specific order.

So it didn't ring the bell...

Share this post


Link to post
Share on other sites

The order of evaluation of function arguments has always been unspecified. This isn't a gotcha so much as just depending on unspecified behavior.


http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4228.pdf

Doesn't completely negate problems with evaluation order, but it fixes the more serious problems with it.



I hate that you can't take a pointer to constructors and destructors.


For constructors:
 
#include <memory>

// if you're being forced at gunpoint to use a C function pointer for some reason
template <typename T, typename... Params>
void constructor_for_c_function_pointers(void* memory, Params... params)
{
  new (memory) T{std::move(params)...};
}

// better version with perfect forwarding for when you're using C++ algorithms/types
template <typename T, typename... Params>
void constructor(void* memory, Params&&... params)
{
  new (memory) T{std::forward<Params>(params)...};
}
For destructors:
 
template <typename T>
void destructor(T* object) { object->~T(); }
You can make plain ol' C function pointers to the destructor version.

Any mediocre compiler with optimize out those thunks, of course.
 

Also a lot of the time there's a huge separation between allocation and initialization, though constructors attempt to package this into a single piece of code.


I'll admit that it's mildly annoying that you can't invoke constructors directly like you can with destructors but placement-new makes it nothing more than a syntactical oddity (which is why it hasn't been 'fixed'; it's just not important to anyone but academic purists).
 

Lastly C++ sucks when you define a constructor and yet want to create an array of your object on the stack and you get the error saying that's not possible without a trivial default constructor.


Incorrect.
 
struct non_trivial
{
  non_trivial(int, float);
};

int main()
{
  non_trivial data[2] = {{1, 2.f}, {2, 3.f}};
}
If you just want to allocate storage and manually construct later:
 
int main()
{
  std::aligned_union_t<0, non_trivial> data[5];

  new (&data[2]) non_trivial(2, 4.f);
}
More convenient wrappers for that abound: uninitialized_array.

Share this post


Link to post
Share on other sites


In this class, a is declared before b and therefore a is also initialized with twice the value of b, before b has been initializer. Code in the constructor is executed top-down, but that code is executed after the initializer list.

 

got it, i read up on initializer lists. now i understand what you're talking about.

 

 


"named constructors" 

 

yep, that's the kind of thing i'm thinking of, some sort of explicit constructor i can call repeatedly, instead of destroying and recreating an object just to re-initialize it for re-use.

 


Every public method should make sure to leave your object in a valid state satisfying the class invariant.

 

i agree totally. but i personally prefer:

1. methods that by design can't cause an invalid state, or

2. by design not passing parameters to a method that will cause an invalid state

as opposed to additional run time checks (when possible).

especially in the called code. i would think it would be better to perform such checks in the calling code before calling the object method. if your parameters are whacked, odds are you don't want to make the call anyway and want to somehow recover instead.

the easiest bugs to fix are the ones you prevent by design (or coding convention w/ strict coder discipline).

an ounce of prevention can prevent a pound of cure. likewise, an ounce of design can sometimes prevent a pound of runtime checks. <g>.

Share this post


Link to post
Share on other sites

by design not passing parameters to a method that will cause an invalid state
as opposed to additional run time checks (when possible).
especially in the called code. i would think it would be better to perform such checks in the calling code before calling the object method. if your parameters are whacked, odds are you don't want to make the call anyway and want to somehow recover instead.
the easiest bugs to fix are the ones you prevent by design (or coding convention w/ strict coder discipline).
an ounce of prevention can prevent a pound of cure. likewise, an ounce of design can sometimes prevent a pound of runtime checks. <g>.

A debug-only assertion that the parameters are valid helps catch bugs (inconsistent calling code checks) and can remind you of these design choices later when you go to call a member function from a new location. Edited by rip-off

Share this post


Link to post
Share on other sites

One thing that tripped me up in the early days of learning c++ was calling virtual functions in constructors.

Do not call virtual functions in constructors, as they will not work properly!

 

happy.png

Depends on what you mean by "not work properly" though. Calling virtual functions in constructors is well defined and well behaved by the language, but may not be obvious or work how you initially expect it work. I suspect that the problem was a lack of understanding of what will happen, rather than it "not working properly".

Share this post


Link to post
Share on other sites

One thing that tripped me up in the early days of learning c++ was calling virtual functions in constructors.
Do not call virtual functions in constructors, as they will not work properly!
 
happy.png

Depends on what you mean by "not work properly" though. Calling virtual functions in constructors is well defined and well behaved by the language, but may not be obvious or work how you initially expect it work. I suspect that the problem was a lack of understanding of what will happen, rather than it "not working properly".


It is hard to argue that the way C++ works in that regard as "intuitive", however, unless you are familiar with a lot of the "dusty corners" of the language. As such, to preserve your own sanity and the sanity of those people trying to read your code it should probably be avoided if at all possible smile.png (This applies to destructors too, which also have the same "problem")

It's also why people don't recommend giving virtual function parameters default values - it doesn't do what most people expect it to.

Share this post


Link to post
Share on other sites


A debug-only assertion that the parameters are valid helps catch bugs (inconsistent calling code checks) and can remind you of these design choices later when you go to call a member function from a new location.

 

yes, an excellent way to help trap out those you don't catch by inspection. unfortunately, you have to throw bad data at the method for it to show up. and if you don't happen to do that before release, you may still have issues later.   

 

it seems that inspection, perhaps augmented by parameter validation, is the most reliable tool we have for making sure code is bullet proof. of course this requires source access. something we may not have.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

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

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

Sign in to follow this