Auto destruct

A

Andre Kostur

JKop said:
Here it is again, without the typos. This one compiles:
[snip code example]

Yuck. Too much manual intervention. The class should be able to manage
whether it should "destroy" the object or not, not the user (OK, not
directly). Model it after std::auto_ptr. Or better yet, get a hold of the
Aug 2001 issue of the C/C++ User's Journal and read the "Generalizing the
Concepts Behind auto_ptr" article by Cristian Vlasceanu.
 
J

JKop

The latest (sorry about the non-standard header):


#include <iostream>
#include <windows.h>

template<class T, void (&CleanUp)(T&,bool&)>
class AutoDestructive
{
private:

AutoDestructive(const AutoDestructive&);
AutoDestructive& operator=(const AutoDestructive&);
AutoDestructive* operator&();
const AutoDestructive* operator&() const;

private:

mutable T t;

public:

T& t_ref;

mutable bool to_be_auto_destructed;

AutoDestructive() : t(), t_ref(t), to_be_auto_destructed(false) {}

~AutoDestructive() { ManualCleanUp(); }

T& operator()() { return t_ref; }
const T& operator()() const { return t_ref; }

void ManualCleanUp()
{
CleanUp(t,to_be_auto_destructed);
}
};

void CleanUp_HKEY(HKEY& hkey, bool& to_be_closed)
{
if (to_be_closed)
{
RegCloseKey(hkey);

to_be_closed = false;
}
}

void CleanUp_HDLL(HMODULE& hdll, bool& to_be_freed)
{
if (to_be_freed)
{
FreeLibrary(hdll);

to_be_freed = false;
}
}

typedef AutoDestructive<HKEY,CleanUp_HKEY> AutoDestructive_HKEY;

typedef AutoDestructive<HMODULE,CleanUp_HDLL> AutoDestructive_HMODULE;


int main()
{
AutoDestructive_HKEY hkey;

LONG error_code = RegOpenKeyEx(HKEY_CLASSES_ROOT,
"*",
0,
KEY_QUERY_VALUE|KEY_SET_VALUE,
&hkey());



if (error_code == ERROR_SUCCESS)
{
hkey.to_be_auto_destructed = true;
}

//Some suff, exception might get thrown here

hkey.ManualCleanUp();

AutoDestructive_HMODULE hdll;

hdll() = LoadLibrary("Monkey");
if(hdll()) hdll.to_be_auto_destructed = true;

//some stuff, exception may be throw

hdll.ManualCleanUp();
}


Next step is contructors...

-JKop
 
A

Alf P. Steinbach

* JKop:
Here it is again, without the typos. This one compiles:

#include <iostream>
#include <windows.h>

Although platform-specific: for C++ you need to #define STRICT and
NOMINMAX before including <windows.h>.

Above gives you declarations for C, not for C++ (much of it works
but some of it doesn't).


template<class T, void (&CleanUp)(T&,bool&)>

You might want to take a look at boost::shared_ptr as well as
ScopeGuard.

More concrete: passing a bool "should something be done or not"
argument to the CleanUp function is not a good idea.

Instead the class should make that decision and call the CleanUp
function or not.

class AutoDestructive
{
private:

AutoDestructive(const AutoDestructive&);
AutoDestructive& operator=(const AutoDestructive&);
OK.

AutoDestructive* operator&();
const AutoDestructive* operator&() const;

AFAICS these are unnecessary.

private:

mutable T t;

Guideline: use 'mutable' only (and if at all) for data members
that can be changed without changing externally observable state.
public:

T& t_ref;

mutable bool to_be_auto_destructed;

See previous comment.

Here it's more acute: don't use 'public' data members for non-trivial
class, and absolutely not 'mutable' -- when you declare an object
const you don't do that because you're OK with that object changing.

AutoDestructive() : t(), t_ref(t),
to_be_auto_destructed(false) {}

Constructor should initialize data members to useful values,
not a default null-state.

~AutoDestructive() { CleanUp(t,to_be_auto_destructed);
}

T& operator()() { return t_ref; }
const T& operator()() const { return t_ref; }
};

Here's an idea that has worked for me many times:

Try _first_ to encapsulate the kind of object or handle in question
(here: registry handle) in a class that provides a complete operation
set -- no "native" stuff in the external interface.

Only then see if you can lift out critical parts as e.g. a reusable
AutoDestructive class.
 
D

David Rubin

JKop said:
The latest (sorry about the non-standard header):


#include <iostream>
#include <windows.h>

template<class T, void (&CleanUp)(T&,bool&)>
^^^^^^^^^^^^^^^^^^^^^^^^^

If you make this a (regular?) parameter (e.g., to the contructor)
rather than a template parameter, you will only instantiate the
template for every T, not for every (T,CleanUp) pair.
class AutoDestructive
{
private:

AutoDestructive(const AutoDestructive&);
AutoDestructive& operator=(const AutoDestructive&);
AutoDestructive* operator&();
const AutoDestructive* operator&() const;

private:

mutable T t;

public:

T& t_ref;

mutable bool to_be_auto_destructed;

AutoDestructive() : t(), t_ref(t), to_be_auto_destructed(false) {}

~AutoDestructive() { ManualCleanUp(); }

T& operator()() { return t_ref; }
const T& operator()() const { return t_ref; }

void ManualCleanUp()
{
CleanUp(t,to_be_auto_destructed);
}
};

IIRC, someone pointed out in an earlier version of this code that
using 'to_be_auto_destucted' is very unnatural, especially since you
require a free function which takes this parameter. The class should
control whether or not the cleanup occurs. Here is an alternative:

template <typename T>
class my_RefProctor {
// This class provides exception safety for an
// object of type T, held by reference.

public:
typedef void (*Callback)(T&); // cleanup callback

private:
T& d_object; // proctored object (held)
Callback d_callback; // user-specified callback
int d_cleanup; // true if cleanup should be done

private:
// not implemented
my_RefProctor(const my_RefProctor&);
my_RefProctor& operator=(const my_RefProctor&);

public:
my_RefProctor(T& object, Callback callback);
// Create a 'my_RefProctor' object to provide exception
// safety for the specified 'object' using the specified
// 'callback' function. The behavior is undefined unless
// 'object' and 'callback' are valid objects.

~my_RefProctor();
// Perform some cleanup for the proctored object by
// calling the stored callback function if 'd_cleanup'
// is 'true'.

void release();
// Release the maintained object from this proctor; no
// cleanup will be done.
};
void CleanUp_HKEY(HKEY& hkey, bool& to_be_closed)
{
if (to_be_closed)
{
RegCloseKey(hkey);
to_be_closed = false;
}
}

typedef AutoDestructive<HKEY,CleanUp_HKEY> AutoDestructive_HKEY;
[snip]
int main()
{
AutoDestructive_HKEY hkey;

LONG error_code = RegOpenKeyEx(HKEY_CLASSES_ROOT,
"*",
0,
KEY_QUERY_VALUE|KEY_SET_VALUE,
&hkey());

if (error_code == ERROR_SUCCESS)
{
hkey.to_be_auto_destructed = true;
}

//Some suff, exception might get thrown here

hkey.ManualCleanUp(); [...]
}

This becomes

int main()
{
HKEY hkey;

LONG rc = RegOpenKeyEx(HKEY_CLASSES_ROOT,
"*", 0, KEY_QUERY_VALUE|KEY_SET_VALUE, hkey);
if (rc != ERROR_SUCCESS) {
// handle error
}

my_RefProctor<HKEY> proctor(hkey, RegCloseKey);

// Some stuff which may throw an exception

proctor.release(); // no exceptions

// further processing

RegCloseKey(hkey);
return 0;
}

This is much more natural as it does no assignments to public members,
does not use or require application-layer wrapper functions, and
conforms to common library usage rather than executing library APIs
through some indirect method.

HTH, /david
 
J

JKop

David Rubin posted:
If you make this a (regular?) parameter (e.g., to the contructor)
rather than a template parameter, you will only instantiate the
template for every T, not for every (T,CleanUp) pair.

I want to be able to do the following:

void CleanUp_HKEY_Normal(T& hkey,bool& to_be_closed) {}

void CleanUp_HKEY_Special(T& hkey,bool& to_be closed) {}

AutoDestructive<HKEY,CleanUp_HKEY_Normal> hkeynormal;
IIRC, someone pointed out in an earlier version of this code that
using 'to_be_auto_destucted' is very unnatural, especially since you
require a free function which takes this parameter. The class should
control whether or not the cleanup occurs.


I made it so for added flexibility. Consider that in your CleanUp function,
even if the key isn't to be closed, you may want to close the entire
registry itself.


-JKop
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
474,174
Messages
2,570,940
Members
47,486
Latest member
websterztechnologies01

Latest Threads

Top