Factories, handles, and handle wrappers
Michael Mol
allocates. You pass these handles to the factory whenever you want to
use the object or destroy it. The factory's "OperateOnObject" class
verifies that the handle is within its object collection, and then
marshals the OperateOnObject call to the class's internal
OperateOnObject method. All external operations on the object
instance are required to pass through the Factory in order to
guarantee that the object instance is still valid at the time of call.
This leads to code roughly like this. (Not using exceptions here, but
rather return error codes.)
// For the sake of scope, consider this as a global singleton factory.
// For the sake of concerns over initialization and destruction,
assume that's dealt with in code not shown.
class SomeFactory
{
public:
OBJHANDLE CreateObject();
ERRCODE DestroyObject(OBJHANDLE);
ERRCODE OperateOnObject(OBJHANDLE objHandle, int someArgument);
protected:
OBJCOLLECTION objCollection;
} factoryObj;
// In some function, somewhere
OBJHANDLE objHandle = factoryObj.CreateObject();
factoryObj.OperateOnObject(objHandle, 42);
factoryObj.DestroyObject(objHandle);
Providing objHandle as the first argument of every call to a distinct
marshalling class instance proves tedious, so a means of overloading
OBJHANDLE is desired such that explicit references to factoryObj are
unnecessary. The resulting OBJHANDLE (a wrapper for the type formally
known as OBJHANDLE) would not likely behave specially at construction
or destruction, nor likely have a custom copy constructor.
The two approaches under consideration are:
OBJHANDLE objHandle = factoryObj.CreateObject();
objHandle->OperateOnObject(42);
// or
(*objHandle)OperateOnObject(42);
objHandle->DestroyObject();
and an approach which is used as follows:
OBJHANDLE objHandle = factoryObj.CreateObject();
objHandle.OperateOnObject(42);
objHandle.DestroyObject();
Both approaches would have the exact same effects on the state of
factoryObj and the members of OBJCOLLECTION, which would also be the
same as the old code where the old OBJHANDLE type was passed as the
first argument.
The question is, which approach is more appropriate and intuitive for C
++, and why? I ask because this is a matter of matter of debate
between another coworker and I, and the third coworker consulted had
no opinion.
--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
Jonathan Lee
> The two approaches under consideration are:
>
> OBJHANDLE objHandle = factoryObj.CreateObject();
> objHandle->OperateOnObject(42);
> // or
> (*objHandle)OperateOnObject(42);
> objHandle->DestroyObject();
>
> and an approach which is used as follows:
>
> OBJHANDLE objHandle = factoryObj.CreateObject();
> objHandle.OperateOnObject(42);
> objHandle.DestroyObject();
>
> ++, and why? I ask because this is a matter of matter of debate
> between another coworker and I, and the third coworker consulted had
> no opinion.
I have two thoughts:
1) The second example is an odd sort of indirection. Are all your
methods called "DoSomethingToObject"? It seems like your handler
class does nothing except "hold" the object. Which leads me to
think
2) If you're going to use indirection, use dereference. But change
the method names. Instead of
objHandle.OperateOnObject(42);
this seems much more natural:
objHandle->Operate(42);
The notation suggests you are acting on the held object. No
need for "OnObject" or similar.
So if you insist on keeping the method names, the "." notation makes
more sense. If you're willing to change, "->" would be even better.
--Jonathan
Maxim Yegorushkin
In other words, you've got:
1) A factory that creates objects.
2) Those objects implement an interface, which is currently belongs to
factory class.
3) You'd also like for the factory to check whether the object reference
is valid.
You can refactor this to simply things.
1) Extract object interface from the factory.
struct SomeObject {
// former SomeFactory::OperateOnObject
virtual ERRCODE Operate(int someArgument) = 0;
virtual ~SomeObject() = 0;
};
Using such object now does not require a factory object, i.e. you can
call Operare() directly on the object.
2) Make factory return smart-pointers to SomeObject. The objects it
creates implement SomeObject interface.
typedef boost::shared_ptr<SomeObject> SomeObjectPtr;
class SomeFactory {
public:
SomeObjectPtr createSomeObject();
...
};
Now the factory function returns a smart-pointer. This smart-pointer
takes care of destroying the object when it is no longer used. No manual
object destruction required.
3) Using a smart-pointer makes the possibility of using an already
destroyed object highly unlikely. Checking whether the object reference
is valid may be not necessary any more.
New usage:
SomeFactory factory;
// later in some function or scope
{
SomeObjectPtr object = factory.createSomeObject();
object->Operate(123);
} // now object goes out of scope and gets destroyed automatically
This looks to be simpler and more intuitive, isn't it?
--
Max
Anders Dalvander
> The question is, which approach is more appropriate and intuitive for C
> ++, and why?
Each class and function should have responsibility for a single task
or related tasks.
The factory class should have the responsibility to create objects.
The OBJHANDLE class should have the responsibility to perform its
specific tasks.
The destructor of a class should have the responsibility to clean up
resources and such.
For instance:
{
some_smart_ptr<OBJHANDLE> obj(factoryObj.CreateObject());
obj->OperateOnObject(42);
// some_smart_ptr is responsible to delete the object, the
destructor of OBJHANDLE is responsible to clean up its resources.
}
Regards,
Anders Dalvander
James Kanze
> Let's say you have a factory which returns handles to objects that it
> allocates. You pass these handles to the factory whenever you want to
> use the object or destroy it. The factory's "OperateOnObject" class
> verifies that the handle is within its object collection, and then
> marshals the OperateOnObject call to the class's internal
> OperateOnObject method. All external operations on the object
> instance are required to pass through the Factory in order to
> guarantee that the object instance is still valid at the time of call.
> This leads to code roughly like this. (Not using exceptions here, but
> rather return error codes.)
This is overly complicated; most importantly, it renders client code
unnecessarily complicated. The simple solution for this is:
1. Install the Boehm garbage collector and use it for these objects.
2. Define a flag in each object, which is set in the constructor, and
reset in the destructor.
3. Check this flag each time you use the object.
--
James Kanze
Michael Mol
I should have used different method names for the example; The ones
given were intended to be generic, but I should have used somthing
similar to DoSomething(). The actual method names in place are closer
to, e.g. GetThroughput(), AttachListener(), DetachListener(), etc.
OBJHANDLE itself is currently actually implemented as a pointer to the
class in question, but typedef'd in a way that direct dereferencing is
impossible (to force checking for validity, rather than risk
dereferencing a NULL, destroyed or otherwise invalid pointer). The
container class in question, as written, contains that value.
If I'm reading your post correctly, then since the wrapper class
doesn't contain the object itself, but rather access to the object,
then you would suggest that the -> syntax is preferred.
j...@jpostmus.com
On 12/16/2009 8:50 AM, Michael Mol wrote:
> OBJHANDLE objHandle = factoryObj.CreateObject();
> factoryObj.OperateOnObject(objHandle, 42);
> factoryObj.DestroyObject(objHandle);
Technically, wouldn't this be
OBJHANDLEWRAPPER objHandleWrapper = factoryObj.CreateObject();
factoryObj.OperateOnObject(objHandleWrapper, 42);
factoryObj.DestroyObject(objHandleWrapper);
Given that, you should be able to operate directly on the wrapper
with:
objHandleWrapper.OperateOnObject(42);
Or, you could extract the handle from the wrapper and then use the
original syntax:
OBJHANDLE objHandle = objHandleWrapper.GetHandle();
factoryObj.OperateOnObject(objHandle, 42);
And obviously you could pass a pointer to the wrapper to another
function, and operate on that with:
SomeClass.SomeFunction(&objHandleWrapper);
void SomeClass::SomeFunction(OBJHANDLEWRAPPER* pObjHndWrpr)
{
pObjHndWrpr->OperateOnObject(42);
}
Just my thoughts...
Michael Mol
> On Dec 16, 2:50 pm, Michael Mol <mike...@gmail.com> wrote:
>
> > The question is, which approach is more appropriate and intuitive for C
> > ++, and why?
>
> Each class and function should have responsibility for a single task
> or related tasks.
Basic OO design. That is exactly what the Factory is doing, in this
case, as it is maintaining a roster of all currently valid instances
identified by OBJHANDLE. The instances themselves have their own
class.
>
> The factory class should have the responsibility to create objects.
> The OBJHANDLE class should have the responsibility to perform its
> specific tasks.
The question I've asked was how to represent the syntax of using the
OBJHANDLE.
> The destructor of a class should have the responsibility to clean up
> resources and such.
The destructor of the class where an instance of which is identified
by OBJHANDLE has the responsibility of cleaning up the class
instance. That hasn't changed.
Actually, I fear I completely FUBAR'd this statement in a total mental
lapse:
"However, the factory
object can't be removed entirely, as it also manages the relationship
of object instances to each other and the system resources they happen
to extract. (A bit of internal behavior I hoped wasn't necessary to
describe in order to ask about intuitive of dereferencing syntax.) "
Those relationships are handled within the class of which that
OBJHANDLE identifies an instance*, not within the factory class.
* Sorry for the obtuse language; OBJHANDLE is intentionally not
directly associated with the type of the class it identifies, because
it's imperative that those class instances not be used without
checking for their validity. If I go ahead and call it a pointer, then
confusion would arise based on the assumption that it's already
directly usable as such. Hence the term "handle". I'm open to
becoming familiarized with clearer language and/or terminology.
Flaamwing
> OBJHANDLE objHandle = factoryObj.CreateObject();
> factoryObj.OperateOnObject(objHandle, 42);
> factoryObj.DestroyObject(objHandle);
>
OBJHANDLEWRAPPER objHandleWrapper = factoryObj.CreateObject();
factoryObj.OperateOnObject(objHandleWrapper, 42);
factoryObj.DestroyObject(objHandleWrapper);
Given that, you should be able to operate directly on the wrapper
with:
objHandleWrapper.OperateOnObject(42);
Or, you could extract the handle from the wrapper and then use the
original syntax:
OBJHANDLE objHandle = objHandleWrapper.GetHandle();
factoryObj.OperateOnObject(objHandle, 42);
And obviously you could pass a pointer to the wrapper to another
function, and operate on that with:
SomeClass.SomeFunction(&objHandleWrapper);
void SomeClass::SomeFunction(OBJHANDLEWRAPPER* pObjHndWrpr)
{
pObjHndWrpr->OperateOnObject(42);
}
Just my thoughts...
John
Michael Mol
> On Dec 16, 9:50 pm, Maxim Yegorushkin <maxim.yegorush...@gmail.com>
> wrote:
> > On 16/12/09 13:50, Michael Mol wrote:
>
> Except that boost::smart_ptr won't necessarily work here---it will
> render destruction non-deterministic, and will cause objects to "leak"
> as soon as there are any cycles.
>
> Of course, his solution won't work either, since without garbage
> collection, there's absolutely no way to ensure that the invalid
> pointer
> remains invalid.
While the identifier space is currently sparse enough that that hasn't
been detected as a problem (normally, fewer than twenty allocations
occur within the lifetime of the application), that's an interesting
point. I originally sought to deal with it in the beginning by using
sequential integer values, but settled back on the raw pointer value
to deal with resulting code complexity. I suppose UUIDs would work,
but would be overkill. A 64-bit sequential integer could work as well;
The run time of the application, and the frequency of allocations,
would mean there would never be an overflow.
Michael Mol
clc++m and now is being brought back here. -mod }
On Dec 17, 2:53 pm, Maxim Yegorushkin <maxim.yegorush...@gmail.com>
wrote:
> On 16/12/09 23:30, Michael Mol wrote:
>
>
>
> > On Dec 16, 4:50 pm, Maxim Yegorushkin<maxim.yegorush...@gmail.com>
> > wrote:
> >> On 16/12/09 13:50, Michael Mol wrote:
>
> >> In other words, you've got:
>
> >> 1) A factory that creates objects.
> >> 2) Those objects implement an interface, which is currently belongs to
> >> factory class.
> >> 3) You'd also like for the factory to check whether the object reference
> >> is valid.
>
> > The code exists, and it's been in the field for over a year. What I'd
> > *like* is an abstracted pass-through to work with existing code.
>
> >> You can refactor this to simply things.
>
> >> 1) Extract object interface from the factory.
>
> >> struct SomeObject {
> >> // former SomeFactory::OperateOnObject
> >> virtual ERRCODE Operate(int someArgument) = 0;
> >> virtual ~SomeObject() = 0;
> >> };
>
> >> Using such object now does not require a factory object, i.e. you can
> >> call Operare() directly on the object.
>
> > Which is the type of apparent behavior I'd like. However, the factory
> > object can't be removed entirely, as it also manages the relationship
> > of object instances to each other and the system resources they happen
> > to extract. (A bit of internal behavior I hoped wasn't necessary to
> > describe in order to ask about intuitive of dereferencing syntax.)
>
> >> 2) Make factory return smart-pointers to SomeObject. The objects it
> >> creates implement SomeObject interface.
>
> >> typedef boost::shared_ptr<SomeObject> SomeObjectPtr;
>
> >> class SomeFactory {
> >> public:
> >> SomeObjectPtr createSomeObject();
> >> ...
> >> };
>
> >> Now the factory function returns a smart-pointer. This smart-pointer
> >> takes care of destroying the object when it is no longer used. No manual
> >> object destruction required.
>
> > The object represents an abstraction of a system resource, and there
> > are a lot of operating factors that weigh in on when it's appropriate
> > to free that system resource. It normally doesn't happen during the
> > weeks-long run of an application, though it does on occasion.
>
> >> 3) Using a smart-pointer makes the possibility of using an already
> >> destroyed object highly unlikely. Checking whether the object reference
> >> is valid may be not necessary any more.
>
> >> New usage:
>
> >> SomeFactory factory;
>
> >> // later in some function or scope
> >> {
> >> SomeObjectPtr object = factory.createSomeObject();
> >> object->Operate(123);
> >> } // now object goes out of scope and gets destroyed automatically
>
> >> This looks to be simpler and more intuitive, isn't it?
>
> > Because of the pervasive and high-traffic use of the subsystem
> > OBJHANDLE is part of, and because of the nature of the application,
> > "highly unlikely" is inevitability, and minimizing risk while meeting
> > the client's feature desires close to their scheduling desires is the
> > order of the day. Given the choice between such a large-scale
> > refactoring and dealing with a new formulation of the code or staying
> > with a tedious syntax, I'd stay with the tedious syntax.
>
> > I understand what you're saying, and for new code design, that would
> > be fine. However, the current code exists, has been fielded for over a
> > year, is a core and frequently-trafficked component where that traffic
> > is fairly sensitive--and it's stable.
>
> > That's why my original fielded question laid out two options; Either
> > can be implemented with few risks to stability, and I was looking for
> > a discussion regarding of wrapping a handle such that it may be
> > operated with a more terse syntax.
>
> I see now.
>
> One possible improvement is to wrap the object handle and a reference to
> a factory in a class, so that its instances can be used as standalone
> objects in the new code that can take advantage of the scoped resource
> management. This can well be the interface shown above.
>
> This way you could have two interfaces to the component -- the existing
> interface and the new one. This allows to gradually evolve to using the
> new interface without breaking the existing code.
>
> The key goal here is to take advantage of RAII / scoped resource
> management. Otherwise you may well be better off using plain C.
If taking advantage of RAII would be beneficial in this case, I'd
definitely look at that. As it is, these particular resources are
usually allocated on application initialization, freed and reallocated
during a runtime system-wide reconfiguration (if such an event
occurs), and are normally release on application shutdown. The
individual instances are continually used during the application's run
(with the exception of during that reconfiguration event).
Even without the runtime configuration, scoped awareness for the
object that OBJHANDLE provides access to isn't beneficial, because the
scope in question would be main()...which is already hidden due to
other frameworks in play.
Michael Mol
> On Dec 16, 1:50 pm, Michael Mol <mike...@gmail.com> wrote:
>
> > Let's say you have a factory which returns handles to objects that it
> > allocates. You pass these handles to the factory whenever you want to
> > use the object or destroy it. The factory's "OperateOnObject" class
> > verifies that the handle is within its object collection, and then
> > marshals the OperateOnObject call to the class's internal
> > OperateOnObject method. All external operations on the object
> > instance are required to pass through the Factory in order to
> > guarantee that the object instance is still valid at the time of call.
> > This leads to code roughly like this. (Not using exceptions here, but
> > rather return error codes.)
>
> This is overly complicated; most importantly, it renders client code
> unnecessarily complicated. The simple solution for this is:
>
> 1. Install the Boehm garbage collector and use it for these objects.
>
> 2. Define a flag in each object, which is set in the constructor, and
> reset in the destructor.
>
> 3. Check this flag each time you use the object.
While I'll admit that the current code has flaws in this area, how
does setting a class member "AmIValid" flag guard against
dereferencing an uninitizialized or wild pointer? In either case, I
have an instant crash. (In most such circumstances, the current code
triggers a hard breakpoint in debug, and a null-op return in release.
The only remaining circumstance hasn't been seen, but is presumably
possible.)
More fundamentally, how does it guard against a stale pointer that
winds up pointing to memory overwritten due to some other heap
allocation? Any non-zero value in that flag would indicate a valid
instance, as far as the code that peeks at that flag is concerned.
(If some of the context is missing, I'm sory; Some of the relevant
replies in comp.lang.c++ don't have comp.lang.c++.moderated in the To
or Followup-To fields.)
James Kanze
It can't do much with regards to a wild pointer (initialized to
some random value). On the other hand, in the above scenario,
the destructor of an object will reset the flag, and garbage
collection will ensure that the underlying memory is not reused.
If all of the member functions check the flag, and if there are
no public data members, all use of a destructed object can be
detected.
> More fundamentally, how does it guard against a stale pointer
> that winds up pointing to memory overwritten due to some other
> heap allocation?
That's why you need garbage collection, point 1 above. After
installing the Boehm collector, you provide a global operator
new function which calls gc_malloc, and a global operator delete
function which does nothing. As long as there is a pointer to
the memory (and sometimes longer), it cannot be reused.
--
James Kanze
Maxim Yegorushkin
> On Dec 17, 1:19 pm, James Kanze<james.ka...@gmail.com> wrote:
>> On Dec 16, 9:50 pm, Maxim Yegorushkin<maxim.yegorush...@gmail.com>
>> wrote:
>>> On 16/12/09 13:50, Michael Mol wrote:
> [snip]
>
>>
>> Except that boost::smart_ptr won't necessarily work here---it will
>> render destruction non-deterministic, and will cause objects to "leak"
>> as soon as there are any cycles.
>>
>> Of course, his solution won't work either, since without garbage
>> collection, there's absolutely no way to ensure that the invalid
>> pointer
>> remains invalid.
>
> While the identifier space is currently sparse enough that that hasn't
> been detected as a problem (normally, fewer than twenty allocations
> occur within the lifetime of the application), that's an interesting
> point. I originally sought to deal with it in the beginning by using
> sequential integer values, but settled back on the raw pointer value
> to deal with resulting code complexity.
That sequential counter would need to be embedded both in the object and
in the handle. Comparing these two integers detects whether the object
being referred to is still the same object. This method, however, does
not protect from accessing an object which exists no more.
> I suppose UUIDs would work,
> but would be overkill. A 64-bit sequential integer could work as well;
> The run time of the application, and the frequency of allocations,
> would mean there would never be an overflow.
To reduce the possibility of overflow that counter only needs to be
incremented on an allocation following a deallocation, since several
allocations in a row with no intervening deallocation always return
unique pointers.
--
Max
Michael Mol
wrote:
> On 18/12/09 05:47, Michael Mol wrote:
>
>
>
> > On Dec 17, 1:19 pm, James Kanze<james.ka...@gmail.com> wrote:
> >> On Dec 16, 9:50 pm, Maxim Yegorushkin<maxim.yegorush...@gmail.com>
> >> wrote:
> >>> On 16/12/09 13:50, Michael Mol wrote:
> > [snip]
>
> >> Except that boost::smart_ptr won't necessarily work here---it will
> >> render destruction non-deterministic, and will cause objects to "leak"
> >> as soon as there are any cycles.
>
> >> Of course, his solution won't work either, since without garbage
> >> collection, there's absolutely no way to ensure that the invalid
> >> pointer
> >> remains invalid.
>
> > While the identifier space is currently sparse enough that that hasn't
> > been detected as a problem (normally, fewer than twenty allocations
> > occur within the lifetime of the application), that's an interesting
> > point. I originally sought to deal with it in the beginning by using
> > sequential integer values, but settled back on the raw pointer value
> > to deal with resulting code complexity.
>
> That sequential counter would need to be embedded both in the object and
> in the handle. Comparing these two integers detects whether the object
> being referred to is still the same object. This method, however, does
> not protect from accessing an object which exists no more.
Having a simple map in the Factory solves the handle->pointer
conversion. (That's actually how I wrote the first draft.) There's
overhead finding the element in the map, but there's overhead in
finding the handle in the existing OBJCOLLECTION.
>
> > I suppose UUIDs would work,
> > but would be overkill. A 64-bit sequential integer could work as well;
> > The run time of the application, and the frequency of allocations,
> > would mean there would never be an overflow.
>
> To reduce the possibility of overflow that counter only needs to be
> incremented on an allocation following a deallocation, since several
> allocations in a row with no intervening deallocation always return
> unique pointers.
So what you're suggesting is creating a UID that's essentially pointer
+incrementor. An interesting idea. In this particular scenario, I
could get away with a 16-bit (or even an 8-bit) incrementor.
--
DeMarcus
> * Sorry for the obtuse language; OBJHANDLE is intentionally not
> directly associated with the type of the class it identifies, because
> it's imperative that those class instances not be used without
> checking for their validity. If I go ahead and call it a pointer, then
> confusion would arise based on the assumption that it's already
> directly usable as such. Hence the term "handle". I'm open to
> becoming familiarized with clearer language and/or terminology.
>
Instead of OBJHANDLE, what about call it an OBJ_PROXY?
http://en.wikipedia.org/wiki/Proxy_pattern
DeMarcus
> directly associated with the type of the class it identifies, because
> it's imperative that those class instances not be used without
> checking for their validity. If I go ahead and call it a pointer, then
> confusion would arise based on the assumption that it's already
> directly usable as such. Hence the term "handle". I'm open to
> becoming familiarized with clearer language and/or terminology.
>
Or maybe you could call it an OBJ_MONIKER.
http://whatis.techtarget.com/definition/0,,sid9_gci293695,00.html
http://msdn.microsoft.com/en-us/library/ms691261%28VS.85%29.aspx