-2

Context: 128kB RAM, freeRTOS.

Considered solutions:

  • Exceptions. Discouraged by both the memory size and the code style guide.
  • Late bool init(...);. Has worked for a decade but has it's problems - can be forgotten or repeated, error status can be ignored.
  • private: bool ok_; which every method checks at it's very beginning and does zero work if false. This would bring a lot of complexity to .cpp file.
  • std::exit() in order to 'fail early, fail big'.

What could improve the readability and robustness beyond the current bool init(...);?

3
  • 5
    In particular, be aware Google's style guide isn't the be all and end all of C++ code; it itself says "Things would probably be different if we had to do it all over again from scratch." Mar 31 at 16:48
  • 3
  • If this is a "can't happen" error, throw the exception. If this is a "you asked for too much memory" or "there's a typo" error, don't. As in the Google Style Guide: "For example, invalid user input should not cause exceptions to be thrown"
    – user949300
    Apr 1 at 17:49

3 Answers 3

0

Almost all the suggestions I see seem to leave kind of the same problem.

Whether init returns a bool, or you have a bool member named ok_, or a factory that returns a null pointer, the code that tries to use the object has to follow pretty much the same pattern:

Try to create an object
Check whether that succeeded
if it succeeded
    do the normal stuff
else
    handle the failure

...and pretty much everywhere you try to use the object, you have similar code:

if we have a valid foo
   use the valid foo
else
   react to not having a valid foo

Whether we encode the failure in a member variable ok_ or in a Boolean return value or a null pointer as a return value, we still end up with pretty much the same basic situation: we've signaled a failure, and now it's up to the calling code to figure out what to handle that failure. None of them makes much real difference in how we write the code.

The factory does (or at least can) help in one specific area though: we can add some logic there to (for example) log the error immediately if initialization fails. But I'd like to go a step further, so all the other places we might use the object have their error handling centralized as well.

As you might guess from the long-winded buildup, I think there's a fairly effective way to do that. One implementation would look something like this:

// ABC purely to define the interface:
class Foo { 
public:
    virtual bool init() = 0;
    virtual void Bar() = 0;
    virtual void Baz() = 0;
};

// This is the real implementation of the `Foo` class. It's pretty much
// like your existing code, but with the ctor make private, and 
// the factory added as a friend.
class GoodFoo : public Foo {
    GoodFoo();
    friend Foo *makeFoo();
public:
    virtual bool init() override { /* same as you've used before */ }
    virtual void Bar() override { /* implementation of Bar */ }
    virtual void Baz() override { /* implementation of Baz */ }
};

// This implementation handles what to do when we would normally call 
// a member function on Foo, but initializing the Foo failed.
// As a simple implementation, we're just going to log the error.
class FailedFoo : public Foo {
    FailedFoo();
    friend Foo *makeFoo();
public:
    // initialization always succeeds:
    virtual bool init() override { return true; }

    virtual void Bar() override { 
        // do whatever we would when we were going to call `Foo::Bar()`, 
        // but initializing the `Foo` failed
        LOG(ERROR) << "Foo::Bar() called after init failed";
    }

    virtual void Baz() override {
        // Likewise: for `Baz()` when initializing `Foo` failed
        LOG(ERROR) << "Foo::Baz() called after init failed";
    }
};

std::unique_ptr<Foo> makeFoo() { 
    auto ret = std::make_unique<GoodFoo>();
    if (ret -> init())
        return ret;
    LOG(ERROR) << "Unable to initialize foo";
    return std::make_unique<FailedFoo>();
}

Now we write the using code to kind of act like creating and initializing a Foo always works:

auto foo = makeFoo(); // either works or logs the error
foo->Bar(); // again, either works, or logs the error

Of course, depending on the situation, Bar or Baz might return something to indicate success or failure--and obviously, the FailedFoo implementation would always signal failure.

1
  • I don't know if this is the best way to handle the situation but for sure it's the most OOP way.
    – Vorac
    May 7 at 4:15
5

Another option is

  • Don't write constructors that can fail.

If you have initialisation that might fail, write a function that can signal failure, and if that succeeds, construct an instance with that state.

I.e. instead of

class ComplexToConstruct {
    int * member
public:
    ComplexToConstruct() : member(new int[100000000]) {} 
    // new might fail 
};

you would instead write

class SimpleToConstruct {
    int * member
    SimpleToConstruct(int * member) : member(member) {} // can't fail
    friend std::optional<SimpleToConstruct> createSimple();
};

std::optional<SimpleToConstruct> createSimple() {
    int * member = new int[100000000];
  // if this fails, we haven't started to construct a SimpleToConstruct
    return member ? SimpleToConstruct(member) : std::nullopt;
}
3
  • A failing constructor in C++, especially one for a global or local variable, or worse for a member of another object, is just asking for trouble. That's because in C++ memory is often allocated for an object; "new Object()" wouldn't have those problems. Swift has "failable initialisers": An initialiser usually returns an object (with a constructor run successfully) but a failable initialiser can return nil which is a very visible failure and handled automatically within other initialisers.
    – gnasher729
    Apr 1 at 9:35
  • 2
    @gnasher729 I don't quite agree. A constructor must produce an object with a valid state or raise an exception. That exceptions don't mesh well with the new-operator is a problem with that operator, and for this (and many other reasons) explicit new is avoided in modern C++.
    – amon
    Apr 5 at 9:54
  • Amon, I mentioned the language. Returning nil as an optional result in Swift is much more likely to be handled properly.
    – gnasher729
    Apr 22 at 12:11
1

A private constructor with a static factory method that returns null in case of failure. For example:

class Widget {
public:
    static Widget* mk(size_t size) {
        if (size == 0) {
            return nullptr;
        }
        if (size > XXXX) {
            return nullptr;
        }
        uint8_t* buf = new(std::nothrow) uint8_t[size];
        if (buf == nullptr) {
            return nullptr;
        }
        Widget* ret = new(std::nothrow) Widget(buf, size);
        if (res == nullptr) {
             delete[] buf;
             return nullptr;
        }
        return ret;
    }


    virtual ~Widget() { delete[] buf_; }
private:
    Widget(uint8_t* buf, size_t size) : buf_(buf), size_(size) {}

    uint8_t *buf_;
    size_t  size_;
};
1
  • 2
    At least use std::unique_ptr for memory-management. Apr 21 at 22:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.