static array declaration in flyweight pattern

P

pauldepstein

I'm concerned about this code which I saw from sourcemaking.com pasted
below. In particular, I question the line -- Icon
*FlyweightFactory::_icons[]; I have marked this line out below by //
************* so that those interested can easily find it in
context. My question is quite simple, and only a tiny fraction of the
pasted code is probably relevant. (But I provided full context just
in case.)

The line Icon *FlyweightFactory::_icons[]; seems a strange definition
of an array. In the class Icon, this array is declared as having
dimension 5 but the definition doesn't specify the size.
The definition of the array also appears to be uninitialized. In
particular, I would be interested to know if the static array
declaration ensures that all the int-pointer members of the array are
null pointers. They are either null pointers or uninitialized
pointers but which?

I would probably have written Icon *FlyweightFactory::_icons[] = {0,
0, 0, 0, 0};

but it's possible that the code does this anyway. Do the int-pointers
initialize to 0 anyway even if this is not made explicit as I did?

Thanks very much for your help.

Paul Epstein


#include <iostream.h>
#include <string.h>

class Icon
{
public:
Icon(char *fileName)
{
strcpy(_name, fileName);
if (!strcmp(fileName, "go"))
{
_width = 20;
_height = 20;
}
if (!strcmp(fileName, "stop"))
{
_width = 40;
_height = 40;
}
if (!strcmp(fileName, "select"))
{
_width = 60;
_height = 60;
}
if (!strcmp(fileName, "undo"))
{
_width = 30;
_height = 30;
}
}
const char *getName()
{
return _name;
}
draw(int x, int y)
{
cout << " drawing " << _name << ": upper left (" << x << ","
<< y <<
") - lower right (" << x + _width << "," << y + _height <<
")" <<
endl;
}
private:
char _name[20];
int _width;
int _height;
};

class FlyweightFactory
{
public:
static Icon *getIcon(char *name)
{
for (int i = 0; i < _numIcons; i++)
if (!strcmp(name, _icons->getName()))
return _icons;
_icons[_numIcons] = new Icon(name);
return _icons[_numIcons++];
}
static void reportTheIcons()
{
cout << "Active Flyweights: ";
for (int i = 0; i < _numIcons; i++)
cout << _icons->getName() << " ";
cout << endl;
}
private:
enum
{
MAX_ICONS = 5
};
static int _numIcons;
static Icon *_icons[MAX_ICONS];
};

int FlyweightFactory::_numIcons = 0;
Icon *FlyweightFactory::_icons[]; //
**********************************************************************************

class DialogBox
{
public:
DialogBox(int x, int y, int incr): _iconsOriginX(x), _iconsOriginY
(y),
_iconsXIncrement(incr){}
virtual void draw() = 0;
protected:
Icon *_icons[3];
int _iconsOriginX;
int _iconsOriginY;
int _iconsXIncrement;
};

class FileSelection: public DialogBox
{
public:
FileSelection(Icon *first, Icon *second, Icon *third): DialogBox
(100, 100,
100)
{
_icons[0] = first;
_icons[1] = second;
_icons[2] = third;
}
void draw()
{
cout << "drawing FileSelection:" << endl;
for (int i = 0; i < 3; i++)
_icons->draw(_iconsOriginX + (i *_iconsXIncrement),
_iconsOriginY);
}
};

class CommitTransaction: public DialogBox
{
public:
CommitTransaction(Icon *first, Icon *second, Icon *third):
DialogBox(150,
150, 150)
{
_icons[0] = first;
_icons[1] = second;
_icons[2] = third;
}
void draw()
{
cout << "drawing CommitTransaction:" << endl;
for (int i = 0; i < 3; i++)
_icons->draw(_iconsOriginX + (i *_iconsXIncrement),
_iconsOriginY);
}
};

int main()
{
DialogBox *dialogs[2];
dialogs[0] = new FileSelection(FlyweightFactory::getIcon("go"),
FlyweightFactory::getIcon("stop"), FlyweightFactory::getIcon
("select"));
dialogs[1] = new CommitTransaction(FlyweightFactory::getIcon
("select"),
FlyweightFactory::getIcon("stop"), FlyweightFactory::getIcon
("undo"));

for (int i = 0; i < 2; i++)
dialogs->draw();

FlyweightFactory::reportTheIcons();
}
 
P

pauldepstein

I'm concerned about this code which I saw from sourcemaking.com pasted
below.  In particular, I question the line -- Icon
*FlyweightFactory::_icons[];    I have marked this line out below by //
*************  so that those interested can easily find it in
context.  My question is quite simple, and only a tiny fraction of the
pasted code is probably relevant.  (But I provided full context just
in case.)

The line Icon *FlyweightFactory::_icons[];  seems a strange definition
of an array.  In the class Icon, this array is declared as having
dimension 5 but the definition doesn't specify the size.
The definition of the array also appears to be uninitialized.  In
particular, I would be interested to know if the static array
declaration ensures that all the int-pointer members of the array are
null pointers.  They are either null pointers or uninitialized
pointers but which?

I would probably have written Icon *FlyweightFactory::_icons[] = {0,
0, 0, 0, 0};

but it's possible that the code does this anyway.  Do the int-pointers
initialize to 0 anyway even if this is not made explicit as I did?

Thanks very much for your help.

Paul Epstein

#include <iostream.h>
#include <string.h>

class Icon
{
  public:
    Icon(char *fileName)
    {
        strcpy(_name, fileName);
        if (!strcmp(fileName, "go"))
        {
            _width = 20;
            _height = 20;
        }
        if (!strcmp(fileName, "stop"))
        {
            _width = 40;
            _height = 40;
        }
        if (!strcmp(fileName, "select"))
        {
            _width = 60;
            _height = 60;
        }
        if (!strcmp(fileName, "undo"))
        {
            _width = 30;
            _height = 30;
        }
    }
    const char *getName()
    {
        return _name;
    }
    draw(int x, int y)
    {
        cout << "   drawing " << _name << ": upper left (" << x << ","
<< y <<
          ") - lower right (" << x + _width << "," << y + _height <<
")" <<
          endl;
    }
  private:
    char _name[20];
    int _width;
    int _height;

};

class FlyweightFactory
{
  public:
    static Icon *getIcon(char *name)
    {
        for (int i = 0; i < _numIcons; i++)
          if (!strcmp(name, _icons->getName()))
            return _icons;
        _icons[_numIcons] = new Icon(name);
        return _icons[_numIcons++];
    }
    static void reportTheIcons()
    {
        cout << "Active Flyweights: ";
        for (int i = 0; i < _numIcons; i++)
          cout << _icons->getName() << " ";
        cout << endl;
    }
  private:
    enum
    {
        MAX_ICONS = 5
    };
    static int _numIcons;
    static Icon *_icons[MAX_ICONS];

};

int FlyweightFactory::_numIcons = 0;
Icon *FlyweightFactory::_icons[];   //
**********************************************************************************

class DialogBox
{
  public:
    DialogBox(int x, int y, int incr): _iconsOriginX(x), _iconsOriginY
(y),
      _iconsXIncrement(incr){}
    virtual void draw() = 0;
  protected:
    Icon *_icons[3];
    int _iconsOriginX;
    int _iconsOriginY;
    int _iconsXIncrement;

};

class FileSelection: public DialogBox
{
  public:
    FileSelection(Icon *first, Icon *second, Icon *third): DialogBox
(100, 100,
      100)
    {
        _icons[0] = first;
        _icons[1] = second;
        _icons[2] = third;
    }
    void draw()
    {
        cout << "drawing FileSelection:" << endl;
        for (int i = 0; i < 3; i++)
          _icons->draw(_iconsOriginX + (i *_iconsXIncrement),
_iconsOriginY);
    }

};

class CommitTransaction: public DialogBox
{
  public:
    CommitTransaction(Icon *first, Icon *second, Icon *third):
DialogBox(150,
      150, 150)
    {
        _icons[0] = first;
        _icons[1] = second;
        _icons[2] = third;
    }
    void draw()
    {
        cout << "drawing CommitTransaction:" << endl;
        for (int i = 0; i < 3; i++)
          _icons->draw(_iconsOriginX + (i *_iconsXIncrement),
_iconsOriginY);
    }

};

int main()
{
  DialogBox *dialogs[2];
  dialogs[0] = new FileSelection(FlyweightFactory::getIcon("go"),
    FlyweightFactory::getIcon("stop"), FlyweightFactory::getIcon
("select"));
  dialogs[1] = new CommitTransaction(FlyweightFactory::getIcon
("select"),
    FlyweightFactory::getIcon("stop"), FlyweightFactory::getIcon
("undo"));

  for (int i = 0; i < 2; i++)
    dialogs->draw();

  FlyweightFactory::reportTheIcons();

}


Sorry, in the above, I said int-pointers. They are of course Icon-
pointers. So my question is whether these pointers are 0.

Thanks,

Paul Epstein
 
M

Marcel Müller

The line Icon *FlyweightFactory::_icons[]; seems a strange definition
of an array. In the class Icon, this array is declared as having
dimension 5 but the definition doesn't specify the size.

This is allowed, since the compiler already knows the size.
The definition of the array also appears to be uninitialized.
Exactly.

In
particular, I would be interested to know if the static array
declaration ensures that all the int-pointer members of the array are
null pointers. They are either null pointers or uninitialized
pointers but which?

I would probably have written Icon *FlyweightFactory::_icons[] = {0,
0, 0, 0, 0};

While this is possible it introduces an implicit dependency on
MAX_ICONS. If MAX_ICONS is less than 5 you will get a compilation error.
Since POD types are automatically initialized to zero in case of an
incomplete initializer list
Icon *FlyweightFactory::_icons[] = {0};
would do the job as well.

but it's possible that the code does this anyway.

Not that I have noticed.
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.


Marcel
 
P

pauldepstein

The line Icon *FlyweightFactory::_icons[];  seems a strange definition
of an array.  In the class Icon, this array is declared as having
dimension 5 but the definition doesn't specify the size.

This is allowed, since the compiler already knows the size.
The definition of the array also appears to be uninitialized.
Exactly.

In
particular, I would be interested to know if the static array
declaration ensures that all the int-pointer members of the array are
null pointers.  They are either null pointers or uninitialized
pointers but which?
I would probably have written Icon *FlyweightFactory::_icons[] = {0,
0, 0, 0, 0};

While this is possible it introduces an implicit dependency on
MAX_ICONS. If MAX_ICONS is less than 5 you will get a compilation error.
Since POD types are automatically initialized to zero in case of an
incomplete initializer list
   Icon *FlyweightFactory::_icons[] = {0};
would do the job as well.
but it's possible that the code does this anyway.

Not that I have noticed.
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.

Marcel

Thanks, Marcel.

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? I incorrectly said that the array holds members of type
int*. Is this a POD type? I didn't think so.
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?
I suppose my questions are:
1) Does the initialization trick Type LargeArray[] = {0}; work
when Type is the int* type?
2) Does the same initialization trick work when Type is the
FairlyComplexClass* type?

Many thanks again for your help?

Paul Epstein
 
B

Bart van Ingen Schenau

The line Icon *FlyweightFactory::_icons[]; seems a strange
definition of an array. In the class Icon, this array is declared
as having dimension 5 but the definition doesn't specify the size.

This is allowed, since the compiler already knows the size.
The definition of the array also appears to be uninitialized.
Exactly.

In
particular, I would be interested to know if the static array
declaration ensures that all the int-pointer members of the array
are null pointers. They are either null pointers or uninitialized
pointers but which?
I would probably have written Icon *FlyweightFactory::_icons[] =
{0, 0, 0, 0, 0};

While this is possible it introduces an implicit dependency on
MAX_ICONS. If MAX_ICONS is less than 5 you will get a compilation
error. Since POD types are automatically initialized to zero in case
of an incomplete initializer list
Icon *FlyweightFactory::_icons[] = {0};
would do the job as well.
but it's possible that the code does this anyway.

Not that I have noticed.
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.

As the array has static storage duration, the array is guaranteed to be
zero-initialised. This means that all array elements will be initialised
to a null-pointer.
Thanks, Marcel.

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?

Yes, it does apply.
I incorrectly said that the array holds members of type
int*. Is this a POD type? I didn't think so.
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?

As int* and Icon* are both plain pointers, the certainly are POD types.
And the trick with specifying ferwer initialisers than there are array
elements works regardless of the element type of the array.
I suppose my questions are:
1) Does the initialization trick Type LargeArray[] = {0}; work
when Type is the int* type?
2) Does the same initialization trick work when Type is the
FairlyComplexClass* type?

Yes to both.
Many thanks again for your help?

Paul Epstein

Bart v Ingen Schenau
 
M

Marcel Müller

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
 

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

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top