RAII and exception in constructor

Question

Imagine that I have a job to do, which can be done in three different ways: a slow and painful, but failsafe way; the way with moderate suffering, given you have Resource1; and a quick and easy way, which requires both Resource1 and Resource2. Now, those resources are precious so I wrap them up into RAII-implementing ResNHolders and write something like this:

void DoTheJob(Logger& log/*, some other params */) {
    try {
        Res1Holder r1(/* arguments for creating resource #1 */);
        try {
            Res2Holder r2(/* arguments */);
            DoTheJobQuicklyAndEasily(log, r1, r2);
        }
        catch (Res2InitializationException& e) {
            log.log("Can't obtain resource 2, that'll slowdown us a bit");
            DoTheJobWithModerateSuffering(log, r1);
        }
    }
    catch (Res1InitializationException& e) {
        log.log("Can't obtain resource 1, using fallback");
        DoTheJobTheSlowAndPainfulWay(log);
    }
}

"DoTheJobXxx()" take references to Logger/ResNHolder, because they are non-copyable. Am I doing it too clumsily? Is there any other clever way to structurize the function?

Anaclinal

I think that is fine. – Gainless

This could sub as a textbook example for try-catch. – Gmur

I would use a factory method that return a optional<Resource> object instead of exception. – Mcnalley

The function names in this are awesome. – Viburnum

Answer 1

I think your code would be just fine, but here is an alternative to consider:

void DoTheJob(Logger &log/*,args*/)
{
    std::unique_ptr<Res1Holder> r1 = acquireRes1(/*args*/);
    if (!r1) {
        log.log("Can't acquire resource 1, using fallback");
        DoTheJobTheSlowAndPainfulWay(log);
        return;
    }
    std::unique_ptr<Res2Holder> r2 = acquireRes2(/*args*/);
    if (!r2) {
        log.log("Can't acquire resource 2, that'll slow us down a bit.");
        DoTheJobWithModerateSuffering(log,*r1);
        return;
    }
    DoTheJobQuicklyAndEasily(log,*r1,*r2);
}

Where the acquireRes functions return a null unique_ptr when the resource fails to initialize:

std::unique_ptr<Res1Holder> acquireRes1()
{
  try {
    return std::unique_ptr<Res1Holder>(new Res1Holder());
  }
  catch (Res1InitializationException& e) {
    return std::unique_ptr<Res1Holder>();
  }
}

std::unique_ptr<Res2Holder> acquireRes2()
{
  try {
    return std::unique_ptr<Res2Holder>(new Res2Holder());
  }
  catch (Res2InitializationException& e) {
    return std::unique_ptr<Res2Holder>();
  }
}

Catafalque

+1 While the original code in the question is correct, this reduces the indentation in the DoTheJob function. It's a matter of taste but I think this is more readable. – Eda

Why is there a unique_ptr around a resource which is already RAII? – Solicitous

@DieterLücking: Making it a pointer allows for the resource to be transferred between functions efficiently and it naturally has a null value to indicate that the resource could not be acquired. Using std::unique_ptr as opposed to using a raw pointer makes sure the resource gets released automatically. – Catafalque

@VaughnCato I know that, but the resource implemented it already! – Solicitous

@DieterLücking: Implemented what? A null state? – Catafalque

@VaughnCato Ok - it's an assumption: ResNHolder has a default constructor – Solicitous

@DieterLücking: If the resource holders have a null state and have the ability to check that state and are movable, then yes the std::unique_ptr isn't necessary. – Catafalque

@DieterLücking You could use an optional here instead of unique_ptr. Which one is better depends on whether the cost of the resources is in stack space consumption(use unique_ptr) or somewhere else(use optional). – Eda

@DieterLücking: Yes, std::optional would be fine as long as the resource holders are movable. – Catafalque

@DieterLücking No, RedNHolder don't have default constructors. If I wanted "probably not initialized resourse", I'd use std::optional. Holders are moveable, of course. – Anaclinal

Answer 2

You code looks fine, the only issue I could imagine you may have with perfomance, as exception considered to be not very effective. If so you may change code to:

void DoTheJob(Logger& log/*, some other params */) {
    Res1HolderNoThrow r1(/* arguments for creating resource #1 */);
    if( r1 ) {
        Res2HolderNoThrow r2(/* arguments */);
        if( r2 ) 
            DoTheJobQuicklyAndEasily(log, r1, r2);
        else {
            log.log("Can't obtain resource 2, that'll slowdown us a bit");
            DoTheJobWithModerateSuffering(log, r1);
        }
    } else {
        log.log("Can't obtain resource 1, using fallback");
        DoTheJobTheSlowAndPainfulWay(log);
    }
}

You would need another RAII object that does not throw exception but has state and returns it in operator bool() or somewhere else. But your code looks less error prone to me, and I would prefer to use it unless you have perfomance issue or need to avoid exceptions.

Artemis