Coding Practice - Float* -> Vector3* cast

Question

If you were given the following:

#include <cassert>

struct vec {
    float x,y,z;
    vec(float x, float y, float z) : x(x), y(y), z(z) {}
};

int main() {
    float verts[] = {1,2,3,4,5,6,7,8,9}; // length guaranteed to be a multiple of 3
    assert(sizeof(vec) == sizeof(float) * 3);

    // Option 1
    for (int i = 0; i < 3; ++i) {
        auto a = reinterpret_cast<vec*>(verts)[i];
        // ...
    }

    // Option 2
    for (int i = 0; i < 9; i += 3) {
        vec a(verts[i], verts[i+1], verts[i+2]); // -O3 averts the copy, so d/w
        // ...
    }

    return 0;
}

Which is considered better practice?

The advantage to the latter option is of course far more explicit, and carries all the advantages/disadvantages that contains; but may not be portable depending on the assertions you can make.

Answer 1

I would go with a 3rd option that is closest to option 2. But instead of taking the three parameters separately in the constructor, take an array of floats.

struct vec {
    float x,y,z;
    vec(float fv[]) : x(fv[0]), y(fv[1]), z(fv[2]) {}
};

int main() {
    float verts[] = {1,2,3,4,5,6,7,8,9};

    for (int i = 0; i < 9; i += 3) {
        const vec a(&verts[i]);
        // ...
    }

    return 0;
}

Answer 2

I strongly prefer option 3.

Options 1 and 2 remind me most of K&R C where a pointer is a pointer is a pointer. If one of the types is changed, the compiler may not notice and that can lead to bugs that may be hard to locate.

Option 3 is the style used in a strongly typed language. If one of the types is changed, the compiler is more likely to notice and complain at the point where code needs to be fixed.

Answer 3

Why not do this:

struct vec
{
    float x,y,z;
};


int main()
{
    vec verts[] = {{1,2,3}, {4,5,6}, {7,8,9}};

It is definitely more clear than any of the proposed solutions.
Is type safe (a key point) and avoids the copy you want (as they are created in place).

// Option 4
for (int i = 0; i < 3; ++i)
{
    const vec& a    = verts[i];

}

When writing C++ code you should definitely not use the C cast operator. C++ has four of its own that you can use:

const_cast           // If you need this you are either very clever or your design is wrong
reinterpret_cast     // This is an indication that you are doing something non portable.
static_cast          // This is a good cast (but any explicit cast is bad)
dynamic_cast         // This casts up and down the class hierarchy (usually just down)

                     // Note: dynamic_cast is a runtime operation.
                     //       all others are compile time casts.
                     //       static_cast/dynamic_cast will actually generate compile
                     //       time errors if you do something wrong very wrong. dynamic_cast
                     //       can also throw an exception (or return NULL).

In your case when I convert your C-casts into C++

   // option 1
   const vec& a = reinterpret_cast<vec&>(verts[i]);

   // Option 2
   const vec& a = reinterpret_cast<vec*>(verts)[i];

Now if you present your code to a C++ developer the first thing they will do is ask: why do you need reinterpret_cast? That's a bad idea.

Answer 4

I'd say, use the last one until this prooves to by a performance problem (which I highly doubt). Also, be aware that the first two solution would modify the array, should the following code modify a.