I need to add logging functionality for an existing code, based on a result of an operation. I was thinking of creating a class that, when constructed, receives a condition function and a function for execution. Based on the result of the condition, the code will be executed.
Example:
class auto_logger{
public:
auto_logger(const std::function<bool()>& cond,
const std::function<void()>& func): _cond(cond), _func(func) {}
virtual ~auto_logger()
{
try {
if (_cond()) {
_func();
}
} catch (...) {
}
}
private:
std::function<bool()> _cond;
std::function<void()> _func;
};
I will use this class like this:
int existing_func()
{
int res = FAILURE;
// this is the only additional code
auto_logger logger([&](){
return res == SUCCESS;
},
[](){
print_to_log();
});
do_some_stuff_that_may_throw();
res = perform_operation();
return res;
}
I would be happy to get some comments on this code. Are there any problem I am not seeing in this solution? Are there any better solutions?
1 Answer 1
Echoing the comments from the question, the usual design for these sort of things, often called a scope guard, is to have a function that is called when it no longer needs to be run (frequenlty called dismiss
or release
.
int existing_func()
{
int res = FAILURE;
auto_logger logger([](){
print_to_log();
});
do_some_stuff_that_may_throw();
res = perform_operation();
dismiss_logger(logger, res);
return res;
}
void dismiss_logger(auto_logger& logger, int result) {
if (res != SUCCESS) {
logger.dont_log();
}
}
virtual ~auto_logger()
Is the expectation that there will be derived classes destroyed via base pointers or via base references? If not, a non-virtual destructor is a common way to signal that a class is not designed to be inherited from.
} catch (...) {
Note that Visual C++ (at least) can be configured via the /EHa switch (this is usually a misguided configuration, IMHO) to catch structured exceptions (things like access violations/segmentation faults) in a catch (...)
handler.
A common technique to hedge against this configuration is to catch std::exception&
under the assumption that all sensible exceptions will derive from it. Anything else being caught (e.g, an int, a structured exception) is likely indicative of a bug in the program so egregious that termination is the only safe recourse, as there's likely some corrupt state somewhere that prevents any further cleanup.
This assumption may or may not be valid. Say this code needs to work in an environment where it is normal and okay to throw and catch things that don't derive from std::exception
. (E.g., a framework that has its own base exception class is being used.) If Visual C++ isn't being used of /EHa will never be used, this recommendation may come off as paranoid. :-)
-
1\$\begingroup\$ Yes the catch must by
...
. Destructors should be designed to never leak any exceptions. If a destructor calls a function you should have a guarantee that it will not throw or you catch everything. stackoverflow.com/a/130123/14065 I also believe you are confusing C++ exceptions and structured exceptions of Windows (these are completely different and not caught by the same mechanism). \$\endgroup\$Loki Astari– Loki Astari2013年11月16日 21:57:56 +00:00Commented Nov 16, 2013 at 21:57 -
\$\begingroup\$ @LokiAstari, thanks. Clarified and scoped more to Visual C++. \$\endgroup\$chwarr– chwarr2013年11月16日 22:20:33 +00:00Commented Nov 16, 2013 at 22:20
std::function
) is necessary, if you use C++11's auto and a function to create the logger. Additionally, the exception handler in the dtor might hide problems. Note boost has a library for this: Boost.ScopeExit (with C++11 support) \$\endgroup\$_
. You need to be careful not to fall into the trap of using a reserved identifier. stackoverflow.com/a/228797/14065 and it looks ugly. \$\endgroup\$