2
\$\begingroup\$

I need to call some functions and clean up some resources acquired after those calls. I can't just call some determined Cleanup function in the end, because during the execution of the functions exceptions may be thrown, so the precise cleanup action depends on the point at which exception is thrown (it may be thrown in the first function, in second, ...). So I created Cleaner helper class, in which I register cleaning actions, specific for each function. If at some point exception is thrown, only cleanup actions that are really needed (and are correct) will be called.

#include <functional>
#include <vector>

using CleanupAction = std::function<void()>; 

class Cleaner
{
public:
    void AddCleanupAction(const CleanupAction &i_action)
    {
        m_cleanup_actions.push_back(i_action);
    }
    ~Cleaner()
    {
        for (auto &action : m_cleanup_actions)
            action();
    }
private:
    std::vector<CleanupAction> m_cleanup_actions;
};

struct SomethingFirstDoer
{
    static void Do()
    {
        // do something first ...
    }

    static CleanupAction GetCleanupAction()
    {
        return []()
        {
            // cleanup needed for the first action..
        };
    }
};

struct SomethingSecondDoer
{
    static void Do()
    {
        // do something second ...
    }

    static CleanupAction GetCleanupAction()
    {
        return []()
        {
            // cleanup needed for the second action..
        };
    }
};

void DoSomething(Cleaner &i_cleaner)
{
    SomethingFirstDoer::Do();
    i_cleaner.AddCleanupAction(SomethingFirstDoer::GetCleanupAction());

    SomethingSecondDoer::Do();
    i_cleaner.AddCleanupAction(SomethingSecondDoer::GetCleanupAction());
}

void DoSomethingWithCleanup()
{
    Cleaner cleaner;
    DoSomething(cleaner);
}

Any remarks, low-level and high-level, would be appreciated.

UPDATE 1:

Another version of the code: now it doesn't use the Cleaner class, just relay on local objects destructors.

#include <functional>

#include <boost/scope_exit.hpp>

using CleanupAction = std::function<void()>; 

struct SomethingFirstDoer
{
    static void Do()
    {
        // do something first ...
    }

    static CleanupAction GetCleanupAction()
    {
        return []()
        {
            // cleanup needed for the first action..
        };
    }
};

struct SomethingSecondDoer
{
    static void Do()
    {
        // do something second ...
    }

    static CleanupAction GetCleanupAction()
    {
        return []()
        {
            // cleanup needed for the second action..
        };
    }
};

void DoSomething()
{
    SomethingFirstDoer::Do();
    BOOST_SCOPE_EXIT(void) {
        SomethingFirstDoer::GetCleanupAction();
    } BOOST_SCOPE_EXIT_END

    SomethingSecondDoer::Do();
    BOOST_SCOPE_EXIT(void) {
        SomethingSecondDoer::GetCleanupAction();
    } BOOST_SCOPE_EXIT_END
}
\$\endgroup\$

closed as off-topic by Toby Speight, Sᴀᴍ Onᴇᴌᴀ, Dannnno, Stephen Rauch, hjpotter92 May 17 '18 at 14:23

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – Toby Speight, Sᴀᴍ Onᴇᴌᴀ, Dannnno, Stephen Rauch, hjpotter92
If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ Can you add a main() that shows how to use this? At present, it's not clear how this is more convenient to users than simple catch blocks to clean up, or RAII objects. \$\endgroup\$ – Toby Speight May 17 '18 at 9:22
  • \$\begingroup\$ How would you recommend to use simple catch blocks, for example? \$\endgroup\$ – undermind May 17 '18 at 9:43
  • \$\begingroup\$ try { action1(); try { action2(); } catch (...) { cleanup2(); throw; } } catch (...) { cleanup1(); throw; } It's ugly, so it would be nice to see how you improve on that. \$\endgroup\$ – Toby Speight May 17 '18 at 10:11
  • \$\begingroup\$ well, we just call DoSomethingWithCleanup() (or DoSomething() for code from Update 1) in main. Sorry, maybe I didn't understand the question. \$\endgroup\$ – undermind May 17 '18 at 10:27
  • \$\begingroup\$ I'm afraid this question does not match what this site is about. Code Review is about improving existing, working code. The example code that you have posted is not reviewable in this form because it leaves us guessing at your intentions. Unlike Stack Overflow, Code Review needs to look at concrete code in a real context. Please see Why is hypothetical example code off-topic for CR? \$\endgroup\$ – Dannnno May 17 '18 at 13:36

Browse other questions tagged or ask your own question.