# Is it a terrible practice to NOT initialize an object in its constructor?

I'm currently working on my own game engine, more precisely on the resources management part.

For now, most of my assets inherits of the following - simplified - class:

class   Asset
{
public:
Asset(const char* file);
virtual ~Asset();

}


As you can guess, the constructor and destructor do almost nothing, while load and unload functions respectively initialize and delete all asset's data.

I'm doing it because I don't want to load a file each time I create an asset object, but instead do it on a method of my AssetsManager that would load everything needed.
But I still want to be able to prepare other objects using these unloaded assets. For instance, an unloaded texture can be set on a mesh material. This material just won't be able to be rendered if the texture is not loaded.

The problem is, I heard this is a terrible way to code a class structure, especially from programmers supporting the RAII idiom.

Is it a correct way to handle assets? If not, what would be better for this class?

• This will most likely depend on the context. If you need instances and can't use (smart?) pointers, and can't initialize on construction, well you're stuck with load/unload methods. IMHO, you should try to do RAII, but there are contexts where you can't do it. Document what you've done and why you've done it in this way, and you should be all right (and perhaps in 2 years from now you'll realize you could have done it in another better way and curse yourself). – Vaillancourt Sep 27 '15 at 4:10
• @AlexandreVaillancourt I added some details of the context. But don't worry, I'm prepared to review my code in a few years to mock myself. – Aracthor Sep 27 '15 at 4:19
• The class is usable, if the behaviour is 'correct' whether the file (resource) is loaded or not, your class behaves correctly. (E.g. if the texture is not loaded, and the mesh is displayed without a texture, and this is the expected behaviour that you have defined for your engine, well, it is RAII, your object is perfectly usable in either cases.) – Vaillancourt Sep 27 '15 at 4:25
• Okay the answer you had addressed very specific C++ stuff, I like this as a systems programmer but it's not really valid from an engine viewpoint as we care about speed, not nice-ness of what we write. – Alec Teal Sep 27 '15 at 20:05

So the other answer is good, but I will say this. In C++ the RAII idiom means "you never type object.close() or object.free() or object.release() or anything like that, it is a bit laxer on initialisation because of assignment operators.

Suppose we have the following

class Object {
public:
Object() { /* CANNOT doStuff after this, as it isn't ready*/ }
Object& operator=(const Object&);
~Object() { /*code to tidy up*/ }

void doStuff();

private:
/*implementation*/
};


I can now do stuff like:

Object o1; //not valid here
Object o2; //careful - you cannot use virtuals in a constructor!
o1=o2; //copy-assign, both valid


When this code-block ends both will automatically be destructed and clean up.

The important part is that the destructor clears up. This means if you have the following:

Asset::~Asset() {
}
}


What you need is a way to communicate "you cannot use an asset that hasn't been loaded" without exceptions (as exceptions degrade performance quite severely)

If you use this then structures filled with assets will automatically unload when you destroy them, as it should be!

## Exemplary uses of RAII

One of my all time favourite C++ things is:

1) code.whatever();
2) {Guard g(resource);
3)     do.this.that(BOTH).with(resource);
4) }
5) more.soForth();


g will be destroyed "at line 4" (proper scoping) thus automatically releasing the lock on resource it holds.

## Game engines don't count

Note also that with game engines we care about performance NOT nice code. We're not systems programmers. A good example of this is when you have a virtual function you might need to call loads of times. It is far faster to have some sort of "type ID" and force a cast to the correct type then jump (without devirtualising) to something than it is to do the virtual call.

This is a total anti-pattern and if I saw a systems programmer doing it I'd probably break their legs. It's okay for us chosen few!

## Why you need "invalid" things

Vectors and arrays are cheep. Even if you're going for maximum performance that default constructor running isn't going to be the bottleneck (letting the compiler see it in the header file will help as then it can inline it and wont have to do a full-on function call) and premature optimisation is evil.

Suppose you have a MyVector<T> which is actually a T[] underneath it all, which you get by doing data=new T[size] - this isn't bad to start with (even though it involves size calls to T::T()), the biggest problem here will be memory allocation (new becomes the enemy) - there is literally no need to optimise this now.

Now when you construct your T[] you need to have a default constructor (linked lists can get away without this as they construct new nodes as needed) and this is "classic C++" in the sense of "I know that the array is initialised" and I encourage you to do this even though it is "not as fast". If this is the bottleneck you can of course use malloc and inplace new everything you need. You just have to be very careful, so worry about this later.

If you change your MyVector implementation from something like:

• data[size+1] = incoming; //will use T::operator= To

• new(data+size+1) (incoming) //will use T::T

Which may require some tweaking if one of your T has an operator= for an obscure class but no constructor. Otherwise nothing will change.

OTHER PITFALL
This one is often missed, right now you have this magic global new - if that goes you'll require an Allocator& to everything that might have to allocate. Keep this in mind. If you do switch to a faster more game-friendly allocator, how will your objects get hold of a reference for it?

## Life without exceptions

In theory stuff like thing.use(invalidAsset) should never happen, and usually when it does you'd have an exception.

Exceptions are the enemy in the engine, this is why you may find in other projects nothing seems to return a T& only T* - this is because you can test a T* for being a nullptr. Like with exceptions though, no one seems to bother.

Anyway, you should ask "how can I make debugging these easier?" the secret is the pre-processor.

Somewhere you need a config file and in it you shall write:

#define MYPROJ_SANITY_CHECK            1
#define MYPROJ_SANITY_CHECK_ASSETS     MYPROJ_SANITY_CHECK


and another file where you define this:

#define MYPROJ_EXIT(MSG) ::MyProject::exit(MSG,__LINE__,__FILE__);
namespace MyProject {
exit(const char* message, int line, const char* file);
}


and make sure the implementation of exit outputs something on stderr and EXITS, not exceptions, insta-crash it! (Great place to set breakpoints!)

Then when you use an Asset (this is the hard part) you turn this:

use(incomingAsset);


into

#if MYPROJ_SANITY_CHECK_ASSETS == 1
}
#endif
use(incomingAsset)


This way you still get to do checks (which you want!) but you can turn them all off easily (which you also want!)

## Real example

template<class T> class DeleteWhenDone;
namespace std {
template<class T>
void swap(DeleteWhenDone<T>&,DeleteWhenDone<T>&) noexcept;
}

template<class T>
class DeleteWhenDone {
public:
DeleteWhenDone() { data = nullptr; }
DeleteWhenDone(T* source): data(source) { }
DeleteWhenDone(const DeleteWhenDone&) = delete;
DeleteWhenDone(DeleteWhenDone&& from) noexcept: data(from.data) { from.data = nullptr; }
DeleteWhenDone& operator=(const DeleteWhenDone&) = delete;
DeleteWhenDone& operator=(DeleteWhenDone&& from) {
DeleteWhenDone tmp(::std::forward<DeleteWhenDone>(from));
::std::swap(*this,tmp);
return *this;
}
~DeleteWhenDone() { delete data; }
private:
T* data;

friend void ::std::swap<>(DeleteWhenDone&,DeleteWhenDone&) noexcept;
};
namespace std {

template<class T>
void swap(::DeleteWhenDone<T>& A, ::DeleteWhenDone<T>& B) noexcept {
T* tmp = A.data;
A.data = B.data;
B.data = tmp;
}

}


The STL isn't enough for engines. https://stackoverflow.com/a/20290701/2112028 (check out this answer on a fast shared pointer, the STL is a systems programming perspective, it's good, but not for engines) So we write our own. This (missing an operator->) class is an example of something that takes whatever it points to with it when it is destroyed. Say you have a vector of these, or an array - when it is destroyed these pointers delete what they point to with it.

Notice the default constructor and the way I implement the move operators, this is very common in systems programming and the compiler will optimise it.

I use this example to illustrate something unexpected. Notice I use nullptr as my "not initialised" value. Compiling with -O3 -fno-exceptions with the following test program

DeleteWhenDone<int> getInt() {
DeleteWhenDone<int> a(new int(5));
return a;
}
int main() {
DeleteWhenDone<int> test;
test = getInt();
}


we get the following code

0000000000400670 <main>:
400670:       53                      push   %rbx
400671:       bf 04 00 00 00          mov    $0x4,%edi 400676: e8 e5 ff ff ff callq 400660 <operator new(unsigned long)@plt> 40067b: 31 ff xor %edi,%edi 40067d: c7 00 05 00 00 00 movl$0x5,(%rax)
400683:       48 89 c3                mov    %rax,%rbx
400686:       e8 85 ff ff ff          callq  400610 <operator delete(void*)@plt>
40068b:       31 ff                   xor    %edi,%edi
40068d:       e8 7e ff ff ff          callq  400610 <operator delete(void*)@plt>
400692:       48 89 df                mov    %rbx,%rdi
400695:       e8 76 ff ff ff          callq  400610 <operator delete(void*)@plt>
40069a:       31 c0                   xor    %eax,%eax
40069c:       5b                      pop    %rbx
40069d:       c3                      retq
40069e:       66 90                   xchg   %ax,%ax


3 calls to delete!! This is BAD (although to a systems programmer this is great)

Earlier when I put the "checking for loadedness in a destructor" I meant it. If we change the destructor to:

    ~DeleteWhenDone() { if(data != nullptr) { delete data; } }


We get the following:

    0000000000400670 <main>:
400670:       48 83 ec 08             sub    $0x8,%rsp 400674: bf 04 00 00 00 mov$0x4,%edi
400679:       e8 e2 ff ff ff          callq  400660 <operator new(unsigned long)@plt>
40067e:       c7 00 05 00 00 00       movl   $0x5,(%rax) 400684: 48 89 c7 mov %rax,%rdi 400687: e8 84 ff ff ff callq 400610 <operator delete(void*)@plt> 40068c: 31 c0 xor %eax,%eax 40068e: 48 83 c4 08 add$0x8,%rsp
400692:       c3                      retq
400693:       66 66 66 66 2e 0f 1f    data32 data32 data32 nopw %cs:0x0(%rax,%rax,1)
40069a:       84 00 00 00 00 00


The last 2 lines are some sort of boundary padding.

The sub at the start moves the stack pointer back 8 bytes (a pointer size) so that the call to operator new returns the result exactly where it'll be needed (on main's stack). At the end it adds it again so when main returns the stack is as expected. You'll see how "truly optimum" this is.

This is the kind of stuff you need to be mindful of.

• This is a great answer as well... But why are exceptions so bad in an engine? For now, I use them as critical errors, so an exception thrown has - in appearance - the same behavior than your MYPROJ_EXIT macro. Is it still evil? – Aracthor Sep 28 '15 at 0:52
• @Aracthor C++ is a "you pay for what you use" language, Exceptions are not "free" I'll put writing about this on my to-do list but today is quite busy. chat.stackexchange.com/rooms/29374/game-tech-internals this is a good chat to find me at. – Alec Teal Sep 28 '15 at 9:16
• – Alec Teal Sep 28 '15 at 14:50

First Approach

A pattern used by several engines is to add either (a) some indirection or (b) extra metadata.

That is, your code can be structured like this (NOTE: all code examples in this post are ad-hoc, probably don't compile, and need a lot of work to make safe and correct):

struct record {
IResource* resource = nullptr;
atomic<int> counter = 0;
};

template <class T> class res_ptr  {
record* rec = nullptr;

public:
explicit res_ptr (record* rec) : rec(rec) { if (rec) ++rec->counter; }
~res_ptr () { if (rec && 0 == --rec->counter) delete rec; }

T* get() const { return static_cast<T*>(rec->resource); }

explicit operator bool() const { return rec && rec->resource; }

// ... supplementary member functions ...
};


That allows you to create a record object when you first request a resource without having to finish loading the resource. You can pass that object around as a shared object managed by the res_ptr<> smart pointer.

This would be used with some kind of resource loader that can create the record object upon request (or return a cached one) and then kick off the resource load on another thread, if you want. Even without threading, this approach lets you unload or recreate the resource object without breaking any references to the record. Note that the code below doesn't deal with uncaching record, which you'd definitely want for correctness.

class loader {
public:
virtual type_index get_type() const = 0;
virtual void* load(string const& path) = 0;
};

class resource_manager {
unordered_map<string, record*> library;

public:
// register a helper that knows how to load a particular type of resource
}

// load a resource of type ResourceT from the given path path
template <class ResourceT>
auto lib_it = library.find(path);
if (lib_it != library.end())
return res_ptr<ResourceT>{lib_it->second};

// attempt to find a registered loader for the resource type
return {};

// create and cache all the record machinery
auto rec = make_unique<record>();
rec->state = make_unique<state>(path);
library.insert({path, rec.get()});

// load the resource now, since we don't have threading in play
return res_ptr<ResourceT>(rec);
}
};


Second Approach

An alternative is to just make your smart pointer store extra metadata. You'd get something like this:

template <class T>
class res_ptr {
T* ptr = nullptr;
string key;

public:
explicit res_ptr (T* ptr) : ptr(ptr) { if (ptr) key = ptr->get_key(); }
explicit res_ptr (string key) : key(std::move(key)) {}
~res_ptr () { if (ptr) resource_singleton.release(ptr); }

T* get() const { return ptr; }
T* get() {
if (!ptr)
return ptr;
}

explicit operator bool() const { return ptr; }
explicit operator bool() { return get(); }

// ... supplementary member functions ...
};


In that case, resource_singleton::try_load will take a given input resource path and check to see if the resource is fully loaded. If the resource is fully loaded then it'll add a reference and return that pointer.

This second approach maps well to some forms of resource loading as you can mark up your objects with res_ptr<> properties; these will get their key loaded off disk and then automatically resolve when necessary. e.g. you might write code like the following and have everything auto-resolve:

// header
class SkinnedMeshComponent {
string name;
res_ptr<Material> mat;
res_ptr<Mesh> mesh;
res_ptr<Skeleton> skel;

REFLECT(SkinnedMeshComponent);
};

// source
BEGIN_REFLECT(SkinnedMeshComponent)
STRING_PROPERTY("name", name)
RESOURCE_PROPERTY("material", mat)
RESOURCE_PROPERTY("mesh", mesh)
RESOURCE_PROPERTY("skeleton", skel)
END_REFLECT(SkinnedMeshComponent)

// data
<object>
<component type="transform" />
<component type="physics" />
<component type="skinned_mesh">
<string key="name">Scary Zombie</string>
<resource key="material">/materials/zombie.mat</resource>
<resource key="mesh">/meshes/zombie.md5</resource>
<resource key="skeleton">/skeletons/zombie.anim</resource>
</component>
</object>

• I'm not sure to fully understand the first method... Doesn't it act a little bit like a singleton for each resource, initializing it on the first call and destroying it when the last using object is deleted? – Aracthor Sep 27 '15 at 8:00
• @Aracthor: kinda. It's initialized when you explicitly hand it a record (which you create when a resource load is requested, even if the resource doesn't exist yet) and then the record is ref-counted. I'll expand the example a bit. – Sean Middleditch Sep 27 '15 at 18:28
• Heya, I've added an answer, but I fear it'll never get seen, can I pester you into taking a look? – Alec Teal Sep 27 '15 at 20:10
• @AlecTeal: seems detailed and more or less correct, and probably a better answer to the specific question being asked. :) – Sean Middleditch Sep 27 '15 at 22:02