Pointers vs references and cleaning it all up

Question

Consider this code:

#include <iostream>
using namespace std;

struct SOME_OTHER_BASE {
    // members
};

struct FOO : SOME_OTHER_BASE {
    int value;
    static FOO *CreatePtr(int param) 
    { 
        FOO *res = new FOO(); 
        res->value = param; 
        return res;
    }
    static FOO &CreateRef(int param)
    {
        return *CreatePtr(param);
    }
};

int main() {
    FOO *ptr = FOO::CreatePtr(2);
    FOO &ref = FOO::CreateRef(4);
    FOO local = FOO::CreateRef(5);

    cout << "ptr: " << ptr->value << endl;
    cout << "ref: " << ref.value << endl;
    cout << "local: " << local.value << endl;

    delete ptr;
    delete &ref;
    return 0;

Requirements:

1. FOO::CreatePtr method may return NULL under some circumstances (allocation failed and was handled, invalid parameters, etc.)
2. FOO is POD, but not an aggregate

What I want: as few as possible delete calls (and memory leaks).

My observations:

1. I can use FOO::CreatePtr, check for NULL and always delete afterwards
2. I can use FOO::CreateRef, but then I can't really check for null and I still have to delete
   1. I could make FOO::CreateRef return a special instance when CreatePtr was NULL. That would help with NULL checking, but still not with deleting.
3. I could use the local variant, but I think I have a memory leak right there (the CreatePtr(5) never gets deleted, only copied into local)
   1. This is highly ineffective because there's an unnecessary alloc and a copy
   2. I can't use brace initialization because FOO is not an aggregate

Is there any way I could declare a local variable of FOO type and initialize it within declaration such that it would automatically get deleted on function exit?

Answer 1

If your object can't be created, throw an exception. It's more useful anyway, as it gives more context why an object couldn't be created and allows the program to fix the problem. Also you avoid the problems with delete because you don't use new. If the type was to be used polymorphically, you would need to use smart pointers like std::unique_ptr or std::shared_ptr.

#include <iostream>
#include <stdexcept>
using namespace std;

struct SOME_OTHER_BASE {
    // members
};

struct CreationFailure : std::runtime_error
{
    CreationFailure(const std::string& s) :
        std::runtime_error(s)
    {

    }
};

struct FOO : SOME_OTHER_BASE {
    int value;

    FOO(int value) : 
        SOME_OTHER_BASE(), // pass the constructor arguments
        value(value)
    {
        // additional actions
        if(/* creation failed */)
            throw CreationFailure("out of memory");
    }
};

int main() {
    FOO local(2);

    cout << local.value << endl;
    return 0;
}

If you can't add constructors, the same logic can be moved to the factory function:

FOO createFoo(int v)
{
    FOO f;
    f.value = v;
    if(/* creation failed */)
        throw CreationFailure("out of memory");
    return f;
}

Answer 2

Not sure if you should use pointers at all for your case.
Moreover, you're not deleting the pointer created to your local variable.

Since FOO should be a POD and depending on its size (which is dependent on SOME_OTHER_BASE's size), you could return it by value from your create method:

struct FOO : SOME_OTHER_BASE {
    int value;
    static FOO Create(int param) {
        FOO foo;
        foo.value = param;
        return foo;
    }
};

int main() {
    FOO local = FOO::Create(2);
    FOO &ref = local;
    FOO *ptr = new FOO(FOO::Create(6));
    // can check if ptr is NULL if you want

    cout << "local: " << local.value << endl;
    cout << "ref: " << ref.value << endl;
    cout << "ptr: " << ptr->value << endl;

    delete ptr;
    return 0;
}