tags:

views:

889

answers:

11

I need to define a class like this:

class Color
{
private:
   union Data
   {
       unsigned int intValue;
       unsigned char argbBytes[4];
   }

private:
    Data m_data;
};

Another way is of course define the data as integer and cast it to char array whenever necessary.

I'm wondering which one is the preferred way. The contradiction here is that I have remote memory of someone's remind not to use union anymore however it seems to be a cleaner solution in this case.

+3  A: 

Using unions is still acceptable practice. Just change rgbBytes to an array :)

In C, unions can be used for different purposes. Sometimes they are used as a Variant type, i.e. to hold values of different type in the same memory location. This usage would be questionable in C++, because you'd use inheritance/polymorphism. However, the other use of unions is to provide different "interface" to the same data. This kind of usage is still valid for C++.

Igor Krivokon
+10  A: 

Unions are fine, as long as you use them carefully.

They can be used in two ways:

  1. To allow a single type of data to be accessed in several ways (as in your example, accessing a colour as an int or (as you probably intended) four chars)

  2. To make a polymorphic type (a single value that could hold an int or a float for example).

Case (1) Is fine because you're not changing the meaning of the type - you can read and write any of the members of the union without breaking anything. This makes it a very convenient and efficient way of accessing the same data in slightly different forms.

Case (2) can be useful, but is extremely dangerous because you need to always access the right type of data from within the union. If you write an int and try to read it back as a float, you'll get a meaningless value. Unless memory usage is your primary consideration it might be better to use a simple struct with two members in it.

Jason Williams
I am not sure if I agree with your case 2. As clearly lyxera is using the union to gain access to the same data with an integer and char array representations, so a struct with two data members would not work.
Nixuz
In case 2 I would definitely use a BOOST.any or BOOST.variant
TimW
+2  A: 

Is it good practice? Yes, but with some caveats.

Back in the days when memory was scarce, unions were popular to re-use memory. Those days are long gone and using unions for that purpose adds needless complexity. Don't do it.

If a union genuinely describes your data, as it does in the example you give, then it is a perfectly reasonable thing to do. However, be warned that you are building in some platform dependencies. On a different platform with different integer sizes or different byte ordering you might not get what you were expecting.

Michael J
inttypes.h with its uint8_t etc. is the right way do this, if the size of data matters (e.g. a wire format), but don't forget about endian issues!
Chris Huang-Leaver
+2  A: 

Since you are using C++, I'd say that it's not good practice. If you are limited to pure C, sure why not.

The biggest problem imo is that the size of the union is always the size of the largest "member", so if you want to store a byte or a shitloadofdata, the size is sizeof(shitloadofdata) and not a byte.

Polymorphism is a far better option than unions.

Magnus Skog
+5  A: 

In C++ the use of unions is constrained by the fact that that their members must be PODs (plain old data). For example, a union member cannot have a constructor or a destructor, among other restrictions.

anon
You can have constructors for unions in C++, as well as methods, virtual functions, etc.
Niki Yoshiuchi
Read what I said - union MEMBERS cannot have constructors etc.
anon
A: 

In my hypothetical C++ Coding Standards unions would be banned, since they tend violate the "correctness, simplicity and clarity come first" rule.

However, this is not the widespread recommendation, and Sutter and Alexandrescu didn't rule against them in their C++ Coding Standards, as far as I remember.

Fortunately, everybody I know finds them so hard to get right that they don't produce them. If only they had found void *'s in APIs hard to get right, too :)

Daniel Daranas
There's lots of things in C++ that should be done only rarely, but which are very useful in very limited circumstances. I think "union" is one of these, although I agree that it should be rare in general. One obvious application would be in decoding byte-level protocols.
David Thornley
A: 

Yes, it is definitely good practice to use unions - it is the only way of telling the compiler that a piece of memory is used to store different types. Using a union maintains static type safety since no reinterpret_cast<> need to be used and makes the intent of the code easier to read.

It is also necessary to use unions when compiling with strict aliasing, in which case the compiler will assume that pointers to different types will never point at the same memory. Strict aliasing is a topic in itself, but in short reading/writing to the same memory through different pointer types when strict aliasing is enabled will often not behave as expected.

Viktor
A: 

If you are using GCC and going to dereference pointers referring to the same location, its better to stick with unions.

Consider the following code:

int main() {
    char rgba[4] = {0xCC, 0xCC, 0xCC, 0};
    int *value = (int*)&rgba;
}

Compiling this code with -Wstrict-aliasing=2 will raise a warning saying that strict aliasing rules were violated. Accessing a value is undefined behavior. On the other hand using union to access some part of another variable isn't a violation of strict aliasing rules.

+1  A: 

The thing I don't like about unions is that they are undiscriminated; they give no info about what the underlying type currently is, and it is very very easy to violate type safety by accessing the wrong side of the union.

Boost::variant solves a lot of these problems. As the documentation points out, union is "nearly useless in an object-oriented environment", while boost::variant gives a very object oriented approach to solving the practical union problems. It's interface is designed to not allow access to the variant unless you are using the proper type, and if the "visitor" pattern they gives compile time errors if the union is extended to include a type you didn't expect.

As for if it is useful; I think so. I've used them to simply large interfaces

 class some_xml_class {
 public:
    void set_property(const string&, const string&);
    void set_property(const string&, const vector<string>&);
    void set_property(const string&, const set<string>&);
    void set_property(const string&, int);

    void set_super_property(const string&, const string&);
    void set_super_property(const string&, const vector<string>&);
    void set_super_property(const string&, const set<string>&);
    void set_super_property(const string&, int);

verses

 class some_xml_class {
 public:
    typedef boost::variant<string, vector<string>, set<string>, int> property_type;
    void set_property(const string&, const property_type&);
    void set_super_property(const string&, const property_type&);

(templates could also be useful here, but let's say the impl was long enough I didn't want to inline it)

Todd Gardner
Too make matters worse union can not contain types which have destructor defined. For this and other reasons the only wise application of unions in C++ is to access low-level types in a different way. In every other place I am definitely toward boot::variant.
lispmachine
A: 

See stackoverflow question on this topic ("What is the strict aliasing rule?")

OJW
+1  A: 

That is absolutely a valid use of unions in C++. Depending on what you want to do, you can change your class to a union so you don't have nesting. Unions can have methods and use inheritance. If that is not possible (there are other data members) then you might want to use an anonymous union like this:

class Color
{
private:
   union
   {
       unsigned int intValue;
       unsigned char argbBytes[4];
   };
public:
    unsigned int GetIntValue() { return intValue; }
};
Niki Yoshiuchi
this is interesting.
lyxera
It also works for structs and classes: union { unsigned int intValue; struct { unsigned char r, b, g, a; }; };
Niki Yoshiuchi