0

I've been putting a game engine together as a project to learn more about c++ for about 8 months now. I've got to a stage where in order to avoid allocating in constructors (in advice from university lecturers), I have virtual setup() methods for all of my objects, and a bool to flag whether the setup method has already been called. However I'm having all sorts of logical errors, and it's a huge pain not being able to do anything in the constructor because of allocation being done in setup. I've also been reading about RAII, and it seems that it might be better to just allocate in the constructor so that I don't have to call a setup method.

How bad is allocating memory via new in a constructor? And should I do so in order to conform to RAII?

Edit- just to clarify, people are pointing out using std::vector containers and the like. I'm talking about allocating memory for more objects, rather than for arrays and things. For example - a Button Object needs to create itself a TransformComponent , an AnimationComponent, and a RenderComponent. Currently, in setup() I would create them using new. Would using smart pointers etc mean I didn't need the new keyword?

The objects I'm creating are being forwarded to a Base Class method called addComponent(Component * ) that would store this component in a std::vector of Component * s, so I can't have these objects being cleaned up at then end of the method/constructor.

I was under the impression that

ButtonClass()
{
    SomeComponent * sc = new SomeComponent ();
    addComponent(sc);
}

Works just fine, but

ButtonClass()
{
    SomeComponent sc = SomeComponent ();
    addComponent(&sc);
}

would cause sc to be cleaned up, and the reference to it passed to addComponent would be a pointer to null memory.

If using a smart pointer negates this then I've misunderstood them, I thought they simply deleted things for me in a neater way than calling new and then delete.

The reasons I've been given to "Never allocate in a constructor" is that if the constructor fails, you can't recover that memory. But in agreement to a comment below, I've always suspected that if this were to happen I'd just terminate anyway....

Figwig
  • 462
  • 3
  • 10
  • 5
    Starting from C++14, there are almost no use-cases where you need raw `new`. There are smart pointers and STL containers for that. If you do however find yourself in a situation where you need to call `new`, the best place to do so is constructor (the RAII idiom you mentioned). Then you just need to follow The Rule of Three (or Five in C++11 and up). – Yksisarvinen Aug 16 '19 at 12:13
  • 2
    Don't use `new` and manual memory management. Get familiar with [smart pointers](https://en.cppreference.com/w/cpp/memory) instead. – πάντα ῥεῖ Aug 16 '19 at 12:14
  • Why do you need allocating any memory at all? – user202729 Aug 16 '19 at 12:18
  • 2
    creating objects and calling a `setup` method afterwards is a big leap back (something like 30 years to the past, when there was no C++). Either you misunderstood what your lectureres told you or the advice was not good. All sorts of logical errors is the price you pay for not using contructors for what they are made for – 463035818_is_not_a_number Aug 16 '19 at 12:19
  • 3
    @πάνταῥεῖ, it doesn't look like a duplicate of either of those questions. The OP is asking whether to allocate **in constructor**, not whether to allocate at all or when to use `new`. I think this question is more about [two stage initialization](http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#nr5-dont-dont-do-substantive-work-in-a-constructor-instead-use-two-phase-initialization) rather than dynamic allocation. – r3mus n0x Aug 16 '19 at 12:38
  • 1
    @r3musn0x: No, the comments make sense/explain why it's a duplicate. `new` shouldn't be in your constructor, but in some resource management class like std::string` or a factory function like `std::make_unique`. – MSalters Aug 16 '19 at 12:43
  • In my experience, university lecturers more often than not give the worst advice imaginable. If you need to allocate in the ctor, just do that. What's gonna happen? You run out of memory? Most people will tell you that if that happens, it's time to terminate the program anyway, doesn't matter if the allocation is in the ctor or not. – Nikos C. Aug 16 '19 at 12:57
  • @MSalters That’s just begging the question. OP still needs to know whether to perform the allocation (via `make_unique` then, I guess) inside the constructor or in a virtual `setup` member function. – Konrad Rudolph Aug 16 '19 at 13:39
  • @KonradRudolph: That's actually another issue - `virtual` likely doesn't work as the asker expects. In the base class ctor, the object under construction still .has the base type which means that calling a `virtual setup` just calls `Base::setup`. If that's pure virtual, that blows up, if it's defined it's probably not intended. – MSalters Aug 16 '19 at 13:41
  • Maybe the lecturers are referring to constructors that do "too much work", i.e. functionality that would be placed in an `execute()`, `run()`, or similar named function, not in a `setup` function. Things like opening and processing files, connecting to ports and sending data, breaking down some huge JSON or XML data and placing it in some structures, etc. would be eligible for "execute" functions, not in a constructor, IMO. – PaulMcKenzie Aug 16 '19 at 13:46
  • @MSalters My point exactly: That’s something that isn’t explained in the duplicates but it’s fairly important for OP. – Konrad Rudolph Aug 16 '19 at 13:46
  • Edited question to make it more specific. My fault for not specifying what I was allocating memory for. – Figwig Aug 16 '19 at 14:03

2 Answers2

4

A good rule of thumb is that each class should have one responsibility. We already have a few (template) classes that are responsible for handling heap-allocated memory, such as std::vector, std::string and std::unique_ptr. Most of the times, you should use those to avoid giving your class an extra memory-handling responsibility.

Now it might happen that you need a specific kind of memory handling that's not offered by a standard class. Or more common, you have some other non-memory resource that also needs cleanup handling - a temporary file for instance. In those cases, the "one class, one responsibility" principle still holds. You wrap each such resource in its own resource-management class. A more complex object that needs three resources just has three members, each of which handles one resource.

C++ now makes sure that even in the presence of exceptions, resources don't get lost. Even if in the middle of the constructor of some complex object one member couldn't be created, C++ will arrange that all members created so far are destroyed, and only those. This gives you an all-or-nothing approach. There are no half-done objects.

To be extra clear: This applies only to constructors. It won't apply to your own setup(). In other words, C++ has specific rules to make resource acquisition work inside constructors, and only there. That's why the term is "Resource Acquisition is Initialisation" or RAII. Your lecturers fundamentally do not understand C++ if they advise against resource allocation in constructors.

MSalters
  • 159,923
  • 8
  • 140
  • 320
  • 1
    The second-to-last paragraph should probably make it clear that “in the middle of the constructor” refers to the initialisation list, rather than to the constructor function body: If the code doesn’t use initialisers and instead reassigns the members, the destructor won’t be aware of this partial initialisation. – Konrad Rudolph Aug 16 '19 at 13:49
  • 1
    @KonradRudolph: Doesn't even matter for correctness. Just take a simple class with two `std::string` members, say `struct Person { std::string name; std::string address; }`. Even if you use assignment in `Person::Person`, it won't ever lose resources. That's of course because those `std::string`s are default-constructed if not mentioned in the initializer list. And that means they are fully constructed, not partially constructed. The only time you have partial construction is when you have an exception while evaluating the initializers. – MSalters Aug 16 '19 at 14:11
  • Ah, if you’re using proper RAII resource handles then you’re of course right. – Konrad Rudolph Aug 16 '19 at 15:46
  • Thanks for the advice. If setup() is called in a constructor, I'm assuming that works too? Your "only there" comment suggested to me that I had to write the code actually in the constructor brackets themselves rather than calling another function within the constructor. Also, the comments about initialisation lists have confused me slightly, can I or can I not use the new keyword within the function body? – Figwig Aug 16 '19 at 17:38
  • @WillHain: That's slightly tricky to answer 100% precise, but it doesn't matter too much for you. If you call `setup` from a constructor, and `setup` throws, then the constructor could catch in theory the error (unless you call `setup` from the initializer list, but that's strange). As for using `new` in a constructor, it is allowed. It's just rarely needed because the Standard already has better wrappers. – MSalters Aug 17 '19 at 00:56
-1

This init() or setup() way is good if you have many constructors doing almost the same thing.

But that is a design error in itself, I think.

So, I would suggest reading your lecturers advice again and if it still means the same to you, then read other peoples opinion, like Scott Meyers.

Constructors are the natural place to allocate resources. I have seen constructors that read in complete databases to set up an application.

Use the smart pointers to allocate things, then you relieved of the duty to keep an eye on them as they are smart enough to take care of themselves.

And have fun!

U. W.
  • 349
  • 1
  • 9
  • 2
    Disagree with the first sentence. If you have many constructors doing almost the same thing, the current solution is constructor forwarding. And even if you _do_ use a common `init` method, it would be `private` and called from each constructor, not `virtual`. – MSalters Aug 16 '19 at 13:26