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?

Was it helpful?

Solution

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>();
  }
}

OTHER TIPS

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.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top