union versus void pointer

In my opinion, the void pointer and explicit casting is the better way, because it is obvious for every seasoned C programmer what the intent is.

Edit to clarify: If I see the said union in a program, I would ask myself if the author wanted to restrict the types of the stored data. Perhaps some sanity checks are performed which make sense only on integral number types. But if I see a void pointer, I directly know that the author designed the data structure to hold arbitrary data. Thus I can use it for newly introduced structure types, too. Note that it could be that I cannot change the original code, e.g. if it is part of a 3rd party library.


I had exactly this case in our library. We had a generic string mapping module that could use different sizes for the index, 8, 16 or 32 bit (for historic reasons). So the code was full of code like this:

if(map->idxSiz == 1) 
   return ((BYTE *)map->idx)[Pos] = ...whatever
else
   if(map->idxSiz == 2) 
     return ((WORD *)map->idx)[Pos] = ...whatever
   else
     return ((LONG *)map->idx)[Pos] = ...whatever

There were 100 lines like that. As a first step, I changed it to a union and I found it to be more readable.

switch(map->idxSiz) {
  case 1: return map->idx.u8[Pos] = ...whatever
  case 2: return map->idx.u16[Pos] = ...whatever
  case 3: return map->idx.u32[Pos] = ...whatever
}

This allowed me to see more clearly what was going on. I could then decide to completely remove the idxSiz variants using only 32-bit indexes. But this was only possible once the code got more readable.

PS: That was only a minor part of our project which is about several 100’000 lines of code written by people who do not exist any more. The changes to the code have to be gradual, in order not to break the applications.

Conclusion: Even if people are less used to the union variant, I prefer it because it can make the code much lighter to read. On big projects, readability is extremely important, even if it is just you yourself, who will read the code later.

Edit: Added the comment, as comments do not format code:

The change to switch came before (this is now the real code as it was)

switch(this->IdxSiz) { 
  case 2: ((uint16_t*)this->iSort)[Pos-1] = (uint16_t)this->header.nUz; break; 
  case 4: ((uint32_t*)this->iSort)[Pos-1] = this->header.nUz; break; 
}

was changed to

switch(this->IdxSiz) { 
  case 2: this->iSort.u16[Pos-1] = this->header.nUz; break; 
  case 4: this->iSort.u32[Pos-1] = this->header.nUz; break; 
}

I shouldn't have combined all the beautification I did in the code and only show that step. But I posted my answer from home where I had no access to the code.

Tags:

C

Unions