How to structure a class with a variable with varying types

[Angus]

I have a class with a member variable which can be of varying types.
For now, I only need to support int and char*.

My first idea was to use a union. So I used:

union{
int nWidget;
char* szWidget;
} m_widget;

void setWidget(int widget)
{
m_widget.nWidget = widget;
}

But I had a problem setting the char*

The ctor for the class set m_widget.szWidget=0

The setter for the char* was like this:
void setWidget(const char* widget, int len = 16)
{
if(m_Widget.szWidget)
delete [] m_Widget.szWidget;

if(len != 0){
m_Widget.szWidget = new char[len];
if(widget)
strcpy(m_Widget.szWidget, widget);
}
}

This is in a class eg class containingwidget.

If I do this:
containingwidget wid;
wid.setWidget(65); //or anything

string strWID("123");
wid.setWidget(strWID.c_str(), strWID.length());

When setWidget is called setting the int, it seems to set szWidget to
65. well in debugger it shows as 0x00000041 (and a blank string -
""). Which seems odd. But problem is the check if(m_Widget.szWidget)
is true and so delete is called when szWidget has not been new'd.


My first question is why am I getting the union problem? And can I
fix it?

Is a union the best way to do this? Any alternative approach? Any
help would be most appreciated.

Angus

 

[Victor Bazarov]

I have a class with a member variable which can be of varying types.
For now, I only need to support int and char*.

My first idea was to use a union. So I used:

union{
int nWidget;
char* szWidget;
} m_widget;

How does your program know which one of the two values is actually the
one that it stored? You need an indicator of sorts:

struct Widget {
enum Type { type_none, type_int, type_charP };
union {
int nWidget;
char* szWidget;
} m_widget;
Type m_widgetType;

Widget() : m_widget(0), m_widgetType(type_none) {}
Widget(int a) : m_widgetType(type_int) { nWidget = a; }
Widget(char const* sz) : m_widgetType(type_charP) {
szWidget = new char[strlen(sz) + 1];
strcpy(szWidget, sz);
}
...
// you have dynamic memory, observe the Rule of Three!
};

That way when you set, you will need to (a) check the type of the
existing data and do what's needed and (b) set the type correctly.

void setWidget(int widget)
{
m_widget.nWidget = widget;
}

But I had a problem setting the char*

The ctor for the class set m_widget.szWidget=0

The setter for the char* was like this:
void setWidget(const char* widget, int len = 16)
{
if(m_Widget.szWidget)

You can't *legally* extract the value that you didn't put there. If you
try accessing the pointer after writing the int, you got undefined behavior.

delete [] m_Widget.szWidget;

if(len != 0){
m_Widget.szWidget = new char[len];
if(widget)
strcpy(m_Widget.szWidget, widget);
}
}

This is in a class eg class containingwidget.

If I do this:
containingwidget wid;
wid.setWidget(65); //or anything

string strWID("123");
wid.setWidget(strWID.c_str(), strWID.length());

When setWidget is called setting the int, it seems to set szWidget to
65. well in debugger it shows as 0x00000041 (and a blank string -
""). Which seems odd.

Why? Whatever is in the memory location 0x00000041 is what it tries to
interpret as a string. The execution environment knows nothing about
the "actual type" of your union. Only you know what you stored last.

> But problem is the check if(m_Widget.szWidget)
is true and so delete is called when szWidget has not been new'd.

Yes, that's the problem. You should not use 'delete[]' when you didn't
allocate anything. The introduction of a type indicator should help you
with that.

My first question is why am I getting the union problem?

Unions are inherently bad to those who decide to deal with them.
Strikes and... Oh, wait... Wrong unions. Anyway...

> And can I
fix it?

Yes, your program has to be made aware of what is contained in your union.

Is a union the best way to do this? Any alternative approach?

Boost? They have some kind of generic 'any' thing, I've never used but
seen suggested here.

V

 

[Pascal J. Bourguignon]

I have a class with a member variable which can be of varying types.
For now, I only need to support int and char*.
When?

My first question is why am I getting the union problem? And can I
fix it?

Because this is not the right solution.

Is a union the best way to do this?
No.

Any alternative approach?

Yes.


The question is when you need to vary the type of this field?


If it's at compilation time, then make this class a template:

template <class WidgetType> class YourClass {
WidgetType widget;
public:
const WidgetType& getWidget(){ return(widget); }
void setWidget(const WidgetType& aWidget){ widget=aWidget; }
// ...
};



If it is at run-time, then wrap the POD in a hierarchy of class types:

class Object {
// ...
};

class Integer : public Object {
// ...
};

class ImmutableCString : public Object {
// ...
};

template <class WidgetType> class YourClass {
Object* widget;
public:
const Object* getWidget(){ return(widget); }
void setWidget(Object* aWidget){ widget=aWidget; }
// ...
};

Of course, as subclasses of Object, you may also have more specific
types, such as a class Widget...

 

[Angus]

Because this is not the right solution.


Yes.

The question is when you need to vary the type of this field?

If it's at compilation time, then make this class a template:

    template <class WidgetType> class YourClass {
      WidgetType widget;
    public:
      const WidgetType& getWidget(){ return(widget); }
      void setWidget(const WidgetType& aWidget){ widget=aWidget; }
      // ...
    };

If it is at run-time, then wrap the POD in a hierarchy of class types:

    class Object {
    // ...
    };

    class Integer : public Object {
    // ...
    };

    class ImmutableCString : public Object {
    // ...
    };

    template <class WidgetType> class YourClass {
      Object* widget;
    public:
      const Object* getWidget(){ return(widget); }
      void setWidget(Object* aWidget){ widget=aWidget; }
      // ...
    };

Of course, as subclasses of Object, you may also have more specific
types, such as a class Widget...

Thank you. I will try your suggestion. Many thanks.

One quick question. In the initialisation of the m_Widget, your ctor
was:
Widget() : m_widget(0), m_widgetType(type_none) {}

But if I use this I get a compile error:
error C2664: '__thiscall
containingwidget::__unnamed::containingwidget::__unnamed(const union
&)' : cannot convert parameter 1 from 'const int' to 'cons
t union &'
Reason: cannot convert from 'const int' to 'const union '
No constructor could take the source type, or constructor
overload resolution was ambiguous

This is using MS VC++ v6. And no I cannot upgrade - work.

How can I init the m_widget to a safe value?

 

[Victor Bazarov]

I have a class with a member variable which can be of varying types.
For now, I only need to support int and char*.

[..]
Of course, as subclasses of Object, you may also have more specific
types, such as a class Widget...

Thank you. I will try your suggestion. Many thanks.

One quick question. In the initialisation of the m_Widget, your ctor
was:
Widget() : m_widget(0), m_widgetType(type_none) {}

Please consider responding to the right person. Pascal's suggestion
wasn't to add type information, and certainly no c-tor like that.

But if I use this I get a compile error:
error C2664: '__thiscall
containingwidget::__unnamed::containingwidget::__unnamed(const union
&)' : cannot convert parameter 1 from 'const int' to 'cons
t union &'
Reason: cannot convert from 'const int' to 'const union '
No constructor could take the source type, or constructor
overload resolution was ambiguous

This is using MS VC++ v6. And no I cannot upgrade - work.

Really? Any work where you have to deal with a 13 year old compiler,
not standard-compliant to boot, sucks. Of course, no work sucks more.

How can I init the m_widget to a safe value?

Don't. Leave it uninitialised.

V

 

[Pascal J. Bourguignon]

One quick question. In the initialisation of the m_Widget, your ctor
was:
Widget() : m_widget(0), m_widgetType(type_none) {}

But if I use this I get a compile error:
error C2664: '__thiscall
containingwidget::__unnamed::containingwidget::__unnamed(const union
&)' : cannot convert parameter 1 from 'const int' to 'cons
t union &'
Reason: cannot convert from 'const int' to 'const union '
No constructor could take the source type, or constructor
overload resolution was ambiguous

This is using MS VC++ v6. And no I cannot upgrade - work.

How can I init the m_widget to a safe value?

By doing what the compiler is telling you to do.
Use a union instead of an integer.

typedef union{
int nWidget;
char* szWidget;
} WidgetUnion;

WidgetUnion m_widget;

Widget(const WidgetUnion& wu): m_widget(wu), m_widgetType(type_none) {};

 

[manish]

I have a class with a member variable which can be of varying types.
For now, I only need to support int and char*.

My first idea was to use a union.  So I used:

union{
 int nWidget;
 char* szWidget;
} m_widget;

How does your program know which one of the two values is actually the
one that it stored?  You need an indicator of sorts:

    struct Widget {
        enum Type { type_none, type_int, type_charP };
        union {
           int nWidget;
           char* szWidget;
        } m_widget;
        Type m_widgetType;

        Widget() : m_widget(0), m_widgetType(type_none) {}
        Widget(int a) : m_widgetType(type_int) { nWidget = a; }
        Widget(char const* sz) : m_widgetType(type_charP) {
           szWidget = new char[strlen(sz) + 1];
           strcpy(szWidget, sz);
        }
        ...
        // you have dynamic memory, observe the Rule of Three!
    };

That way when you set, you will need to (a) check the type of the
existing data and do what's needed and (b) set the type correctly.

void setWidget(int widget)
{
    m_widget.nWidget = widget;
}

But I had a problem setting the char*

The ctor for the class set m_widget.szWidget=0

The setter for the char* was like this:
void setWidget(const char* widget, int len = 16)
{
    if(m_Widget.szWidget)

You can't *legally* extract the value that you didn't put there.  If you
try accessing the pointer after writing the int, you got undefined behavior.

        delete [] m_Widget.szWidget;

    if(len != 0){
        m_Widget.szWidget = new char[len];
        if(widget)
            strcpy(m_Widget.szWidget, widget);
    }
}

This is in a class eg class containingwidget.

If I do this:
containingwidget wid;
wid.setWidget(65); //or anything

string strWID("123");
wid.setWidget(strWID.c_str(), strWID.length());

When setWidget is called setting the int, it seems to set szWidget to
65.  well in debugger it shows as 0x00000041 (and a blank string -
"").  Which seems odd.

Why?  Whatever is in the memory location 0x00000041 is what it tries to
interpret as a string.  The execution environment knows nothing about
the "actual type" of your union.  Only you know what you stored last.

 >  But problem is the check if(m_Widget.szWidget)

is true and so delete is called when szWidget has not been new'd.

Yes, that's the problem.  You should not use 'delete[]' when you didn't
allocate anything.  The introduction of a type indicator should help you
with that.

My first question is why am I getting the union problem?

Unions are inherently bad to those who decide to deal with them.
Strikes and...  Oh, wait... Wrong unions.  Anyway...

 >  And can I

fix it?

Yes, your program has to be made aware of what is contained in your union..

Is a union the best way to do this?  Any alternative approach?

Boost?  They have some kind of generic 'any' thing, I've never used but
seen suggested here.

V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask- Hide quoted text -

- Show quoted text -

boost::variant is best suited for these cases