5

I'm designing a Factory that creates different types of Foo and I'm trying to use smart pointers.

Most seems to be working well, but I am missing some important features (namely nullptr) because of compiler limitations.

I have this method:

std::unique_ptr<Foo> createFoo(const std::string &fooType) {
    auto it = _registeredFoo.find(fooType); // _registeredFoo is a map
    if(it != _registeredFoo.end())
       return std::unique_ptr<Foo>(it->second());
    return std::unique_ptr<Foo>(NULL);
}

When I test this method, it never returns a NULL-like pointer.

This is how I test my method.

std::unique_ptr<Foo> _foo = FooFactory::getInstance()->createFoo("FOO"); //FOO is registered
if(_foo) {
  std::cout << "Hello, World!" << std::endl;
} else {
  std::cout << "Goodbye, World!" << std::endl;
}

std::unique_ptr<Foo> _bar = FooFactory::getInstance()->createFoo("BAR"); // BAR is unregisered
if(_bar) {
  std::cout << "Hello, World!" << std::endl;
} else {
  std::cout << "Goodbye, World!" << std::endl;
}

I always see "Hello, World!"

This leads me to believe that my use of std::unique_ptr's constructor is a bit abusive. Can anyone give me a recommendation for how to approach this without emulating nullptr myself?

I'm using gcc 4.4.6.

erip
  • 16,374
  • 11
  • 66
  • 121
  • 2
    Won't simply using `unique_ptr();` help? – SingerOfTheFall Aug 25 '15 at 12:56
  • 5
    It's actually the same as the [default constructor](http://en.cppreference.com/w/cpp/memory/unique_ptr/unique_ptr). How did you check that your code _"never returns a `NULL`-like pointer"_? – πάντα ῥεῖ Aug 25 '15 at 12:58
  • 6
    _"When I test this method, it never returns a NULL-like pointer."_ This is the only part of your question that is the real question, yet it lacks detail. Can you explain your problem better? – Lightness Races in Orbit Aug 25 '15 at 12:59
  • @JoachimPileborg: Not _much_ surely? All you get is an additional and redundant `delete 0`, no? – Lightness Races in Orbit Aug 25 '15 at 13:00
  • "_When I test this method_..." Please show us your tests! Perhaps your expectation is at fault rather than your code... – Toby Speight Aug 25 '15 at 13:07
  • Edited to include my test. – erip Aug 25 '15 at 13:08
  • After testing a little myself, I'm surprised your code compiles. Passing `NULL` (which in C++ expands to `0`) to the constructor should be ambiguous, as both a `nullptr_t` object can be constructed from the zero, as well as it could be seen as a pointer, so there are two constructors that could be used and the compiler should give you an error. – Some programmer dude Aug 25 '15 at 13:10
  • 9
    GCC 4.4.6 is old. When it says "C++0x support is experimental," it really means it. Upgrade if it's at all possible. You're living on the edge. – Potatoswatter Aug 25 '15 at 13:11
  • @Potatoswatter if only. – erip Aug 25 '15 at 13:11
  • 2
    @erip My fallback advice is to not use C++11. You don't have a C++11 compiler. To "upgrade" the language without the underlying software is just wishful thinking. – Potatoswatter Aug 25 '15 at 13:12
  • 4
    Uh, you are aware that your code will likely lead to double-frees and/or leaked memory, especially if you get `unique_ptr` to work as it should? You pass ownership of the pointed to memory from the registry to the `unique_ptr` you return. The `unique_ptr` will call `delete`, and your registry will probably call `delete`. Same problem if you call `createFoo` with the same argument twice. – ex-bart Aug 25 '15 at 13:19
  • To break down the problem to one step at a time, directly assign an empty `unique_ptr` to `_foo` in your test: `auto foo = std::unique_ptr(NULL);` - vary as needed. Then we can focus on the actual problem. With your archaic g++, we might need to `if (_foo.get())` or some such, even though that should be exactly equivalent. – Toby Speight Aug 25 '15 at 13:20
  • @ex-bart: `it->second()` implies that `it->second` is a factory with a function call operator, and that the factory creates a brand new object. No double-frees or leaked memory. – Lightness Races in Orbit Aug 25 '15 at 14:03

3 Answers3

7

I think that what you want is an empty unique_ptr:

return std::unique_ptr<Foo>();

It seems that you are looking for a pointer that points to no object (and is false when converted to bool).

Toby Speight
  • 27,591
  • 48
  • 66
  • 103
7

You can use the default constructor of std::unique_ptr.

std::unique_ptr<int> a1();
std::unique_ptr<int> a2(nullptr);

a1 and a2 are unique pointers initialised empty (owns nothing).

you can verify the emptiness of the pointers with the operator bool provided by the unique_ptr class:

if (!a1) { // the smart pointer is empty
}
6

What you want is

return {};

the {} return a default constructed object of whatever (class) type1, and can be read "nothing". It works with smart pointers, optional, and most everything.

If your compiler lacks this

return std::unique_ptr<Foo>();

will do it. Passing NULL to std::unique_ptr is not legal in C++11 anyhow (the constructor is ambiguous).

You don't have a C++11 compiler. Any use of C++11 in your code is going to be error-prone.


1 The proper wording is complicated. For a scalar (like int), this zero-initializes the value (which, in practice, sets the value to 0/null/0.0/'\0' etc).

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524