Re: static array declaration in flyweight pattern
pauldepstein@att.net wrote:
Do the int-pointers
initialize to 0 anyway even if this is not made explicit as I did?
No, their values are undefined. But since there is another variable
_numIcons which seems to store the valid part of _icons, this does not
result in undefined behavior.
I have two questions. I realise that POD types are automatically
initialized to 0 in the case of an uninitialized list but does this
apply here?
No, this is implementation dependent. Most multi-tasking OS return only
memory that is initialized to zero to avoid security issues. But this
does non necessarily propagate through the C runtime. Some runtimes
initialize the memory to a magic value for debugging purposes.
I incorrectly said that the array holds members of type
int*. Is this a POD type? I didn't think so.
All pointer types are simple C style data types.
However, the array actually holds members of type Icon* where Icon is
another class. So I don't think the initialization trick you mention
holds, or does it?
This is defined behavior.
I suppose my questions are:
1) Does the initialization trick Type LargeArray[] = {0}; work
when Type is the int* type?
Yes.
2) Does the same initialization trick work when Type is the
FairlyComplexClass* type?
Yes, that makes no difference.
But the coding that you have posted does not rely on either of that,
because it does not read any of the pointers in the array without
writing it before. But the implementation has undefined behavior and is
likely to crash at another point. If you call FlyweightFactory::getIcon
with more than MAX_ICONS different names, the array _icons is accessed
out of it's bounds. So the code is simply a very bad design. At least it
should throw an exception in this case.
If you do C++ I strongly recommend to avoid to use pointers to access
non constant arrays. Move to std::vector instead and all of these
problems disappear. And if your application needs a dictionary like the
posted code you should have a look at std::map or std::set too. They may
do almost all of the work you need except for the construction of new
instances.
Furthermore, you should check who own the objects that you have created
with new. In the example one would expect that FlyweightFactory owns the
Icon instances. But it doesn't. The instances are never destroyed. This
might not be a problem if the Icon class does not rely on its destructor
call. But it might become a problem if Icon controls unmanaged resources
like Windows COM objects (or things that are similarly awful).
With respect to the cleanup, it is more clean to use a static instances
of FlyweightFactory instead of a class with only static functions. Then
you have a destructor call of FlyweightFactory where you can delete the
cache content. If you want to ensure that there are no two instances of
FlyweightFactory in memory, you should consider a singleton patten
implementation.
Marcel