Which is more readable (C++ = )

Question

int valueToWrite = 0xFFFFFFFF;
static char buffer2[256];
int* writePosition = (int* ) &buffer2[5];
*writePosition = valueToWrite;

//OR
* ((int*) &buffer2[10] ) = valueToWrite;

Now, I ask you guys which one do you find more readable. The 2 step technique involving a temporary variable or the one step technique?

Do not worry about optimization, they both optimize to the same thing, as you can see here. Just tell me which one is more readable for you.

or  DWORD PTR ?buffer2@?1??main@@9@4PADA+5, -1
or  DWORD PTR ?buffer2@?1??main@@9@4PADA+10, -1

Answer 1

I find the second, shorter one easier to read.

I suspect, however, that this rather depends on whether you are the type of person that can easily 'get' pointers.

The type casting from char* to int* is a little awkward, though. I presume there is a good reason this needs to be done.

Answer 2

The first example is more readable purely on the basis that your brain doesn't have to decipher the pointer operations globed together.

This will reduce the time a developer looking at the code for the first time needs to understand what's actually going. In my experience this loosely correlates to reducing the probability of introducing new bugs.

Answer 3

int* writePosition = (int* ) &buffer2[5]

Or

*((int*) &buffer2[10] ) = valueToWrite;

Are both incorrect because on some platforms access to unaligned values (+5 +10) may cost hundreds of CPU cycles and on some (like older ARM) it would cause an illegal operation.

The correct way is:

memcpy( buffer+5, &valueToWrite, sizeof(valueToWrite));

And it is more readable.

Answer 4

Watch out -- this code probably won't work due to alignment issues! Why not just use memset?

#include <string.h>
memset(buffer2+10, 0xFF, 4);

Answer 5

If I was forced to choose, I'd say 1. However, I'll note the code as presented is very C like either way; I'd shy away from either and re-examine the the problem. Here's a simple one that is more C++-y

const char * begin = static_cast<char*>(static_cast<void*>(&valueToWrite));
std::copy(begin, begin+sizeof(int), &buffer2[5]);

Answer 6

If you can afford to tie yourself to a single compiler (or do preprocessor hacks around compatiblity issues), you can use a packed-structs option to get symbolic names for the values you're writing. For example, on GCC:

struct __attribute__ ((__packed__)) packed_struct
{
  char stuff_before[5]
  int some_value;
}

/* .... */

static char buffer2[256];
struct packed_struct *ps = buffer2;
ps->some_value = valueToWrite;

This has a number of advantages:

• Your code more clearly reflects what you're doing, if you name your fields well.
• Since the compiler knows if the platform you're on supports efficient unaligned access, it can automatically choose between native unaligned access, or appropriate workarounds on platforms that don't support unaligned access.

But again, has the major disadvantage of not having any standardized syntax.

Answer 7

Once you encapsulate it inside a class, it does not really matter which technique you use. The method name will provide the description as to what the code is doing. Thus, in most cases you will not have to delve into the actual impl. to see what is going on.

class Buffer
{
    char buffer2[256];
public:
    void write(int pos, int value) { 
       int* writePosition = (int*) &buffer2[pos];
       *writePosition = value;
    }
}

Answer 8

Most readable would be either variant with a comment added on what you're doing there.

That being said, I despise variables introduced simply for the purpose of a one-time use a couple of lines later. Doing most of my work in the maintenance area, getting dozens of variable names pushed in my face that are poor efforts not having to write an explanatory comment sets me on edge.

Answer 9

Definitely:

* ((int*) &buffer2[10] ) = valueToWrite;

I parse it not in one but few steps, and that is why it is more readable: I have all steps in one line.