Casting to a base reference and copying is a dirty hack, but what exactly is dirty about it?

Question

Hi all.

Problem: I have a pretty big structure with POD variables, and I need to copy around some fields, but not others. Too lazy to write down a member-by-member copy function.

Solution: move the copy-able fields to the base, assign the base. Like this:

struct A
{
   int a, b, c;
};

struct B : public A
{
    int d, e, f;
};

//And copy:

B x, y;
(A&)x = y; //copies the part of B that is A

Now, this is dirty, I know. I had a live, livid discussion with co-workers re: this code, my competence, and my moral character. Yet the hardest specific charge I heard was "d, e, f are not initialized in the copy". Yes I know; that was the intent. Of course I initialize them elsewhere.

Another charge was "unsafe typecast". But this is a guaranteed-safe typecast to the base class! Is's almost like

((A*)&x)->operator=(b);

but less verbose. The derivation is public; so treating B as A is fair game. There's no undefined behavior, as far as I can see.

So, I'm appealing to the collective wizdom of SO. This is an invitation to criticism. Have a go at it, people.

EDIT: the final line of the snippet can be expanded into less offensive code in more than one way. For example:

void Copy(A& to, const A& from)
{
    to = from;
}

B x, y;
Copy(x, y);

Functionally the same. Or like this:

x.A::operator=(y);

EDIT2: there's no maintenance programmer but me. It's from a hobby project. So stop pitying that poor soul.

Answer 1

Problem: I have a pretty big structure with POD variables, and I need to copy around some fields, but not others. Too lazy to write down a member-by-member copy function.

Then don't. (But then assignment will assign everything.)

Also, if you want slicing, you could do:

B b;
A a;
a = b;

Just having uninitialized members seems dirty indeed.

Answer 2

The problem is that you are initialising a B using an A constructor. I'm surprised compilers allow this.

This means when you go to access the B, and use functions which expect it to have initialised c,d,e, it will not work as expected and may crash. Also if it's actually a class and has virtual functions, it will have a different vtable from that expected by the compiler.

Watch this:

#include <stdio.h>

void f(A& y)
{
  B x;
  x.d = 5;
  printf("%i\n",x.d);
}

void g(A& y)
{
  B x;
  (A&)x = y;                                                  
  printf("%i\n",x.d);                                         
}                                                             

main()                                                        
{                                                             
  B z;                                                        
  z.d = 3;                                                    
  f(z);                                                       
  g(z);                                                       
} 

and compile and run

g++ 1.cc
./a.out
5
5

explain that, please?

Answer 3

Yes; this is dirty - you're intentionally slicing because you're too lazy to write B::CopyVariablesThatIWant(const B&). You're abusing the type system in a way that works for you, however you will most likely confuse and/or enrage any future programmers who have to look at your code and figure out it's intent.

Your coworkers are right, you should be ashamed of yourself.

Answer 4

Why not just do a member struct and do things explicitly:

struct A
{
    int a, b, c;
};

struct B
{
    A top_;
    int d, e, f;
};

//And copy:

B x, y;
x.top_ = y.top_;

And in my opinion the dirtiest part is unnecessary obfuscation of the code. Six month from now some poor soul will curse you trying to understand why in Batman's name it's done this way.

Answer 5

You could get in trouble in the following situation:

struct A {
  char a;
  int b;
};

struct B : public A {
  char c;
};

a is allocated at offset 0 and b is allocated at offset 4 in the structure (padding to align b). c can be allocated at offset 1 because that space is not being used by A. If that happens, your assignment may clobber c (if A's copy constructor copies the unused padding, which it is allowed to do).

I'm not sure whether allocating c in A's padding space is allowed in C++. I wrote a Java compiler that does it, though, so it isn't unprecedented.

Answer 6

What are the semantics that make some fields needed for the copy but not others?

The semantics of operator=() are that afterwards the two objects will be have equivalent observable state. (That is, after a = b;, a == b should return true.) Why you would want to violate those semantics and confuse your maintenance programmers is the real question. What possible long-term benefit do you see to not explicitly writing your MinimalClone() function, versus the long-term harm to ease of understanding your code?

Edit: There's always a maintenance programmer, unless you delete the code just after compilation. I can't count the number of times I've returned to something I wrote months prior and said "what was I thinking?!?" Be kind to your maintenance programmer, even if it's you.

Answer 7

It all depends on the context and on which part is supposedly "dirty" here.

Firstly, the "sliced copying" trick is technically legal. Formally, it is not really a "hack". You can also achieve the same result by using the qualified name of the assignment operator to refer to the operator from A

x.A::operator =(y); // same as `(A&) x = y` in your case

It is starting to look familiar, isn't it? Yes, that's exactly what you would do if you had to implement the assignment operator in the derived class, if you suddenly decided to do it manually

B& B::operator =(const B& rhs) 
{
  A::operator =(rhs); // or `this->A::operator =(rhs)`

  // B-specific part goes here
}

The A::operator =(rhs); part is exactly the same "sliced copying" trick as yours above, however in this case it is used in a different context. Nobody would, of course, blame you for the latter use, since that's how it is normally done and how it should be done. So, again, the "dirtiness" of the specific application of the trick depends on the context. It is perfectly fine as an integral part of the implementation of the derived assignment operator, but it might look highly questionable when used "by itself" as in your case.

However, secondly and more importantly, what I would call "dirty" in your case is not the use of the trick with "sliced copying" itself. From what you described in your question, it looks like you actually split your data structure into two classes (A and B) specifically for the purpose of being able to use the aforementioned trick. This is what I would call "dirty" in this case. Not the "sliced copying" trick itself, but rather the use of inheritance for the sole purpose of enabling the "sliced copying". If you did it just to avoid writing the assignment operator manually, that's an instance of blatant laziness. That's what's "dirty" here. I wouldn't recommend using inheritance for such purely utilitarian purposes.

Answer 8

How about:

struct A
{
    int a;
    int b;
    int c;
};
struct B: public A
{
    int d;
    int e;
    int f;
};

int main()
{
    B   x;
    B   y;
    static_cast<A&>(x)  = y;

}

Answer 9

The big issue I have against this is that it saves you only a little bit of work, and leaves a big "Huh?" for the next guy to come around. Commenting it is no better, as it still leaves something extra to be comprehended and is more typing than just doing it right would be.

After decades of poring over assorted code, I've come to really resent anything that gets in the way of just reading and understanding.