2009-10-07 16 views
12

Considere una clase de la cual las copias deben ser hechas. La gran mayoría de los elementos de datos en la copia deben reflejar estrictamente el original, sin embargo, hay algunos elementos seleccionados cuyo estado no se debe conservar y deben reinicializarse.¿Es malo llamar al operador de asignación predeterminado desde el constructor de copia?

¿Es malo llamar a un operador de asignación predeterminado desde el constructor de copia?

El operador de asignación predeterminado se comportará bien con datos antiguos sin formato (int, double, char, short) así como las clases definidas por el usuario para sus operadores de asignación. Los punteros deberían tratarse por separado.

Una desventaja es que este método deja lisiado al operador de asignación ya que no se realiza la reinicialización adicional. Tampoco es posible deshabilitar el uso del operador de asignación abriendo así la opción del usuario para crear una clase interrumpida utilizando el operador de asignación predeterminado incompleto A obj1,obj2; obj2=obj1; /* Could result is an incorrectly initialized obj2 */.

Sería bueno relajar el requisito de que a a(orig.a),b(orig.b)... se le debe escribir además a(0),b(0) .... Necesidad de escribir toda la inicialización dos veces crea dos lugares en busca de errores y si iban a ser agregado a la clase nuevas variables (digamos double x,y,z), tendría que añadido correctamente en al menos 2 lugares en vez de 1.

código de inicialización ¿Hay una mejor manera?

¿Hay una manera mejor en C++ 0x?

class A { 
    public: 
    A(): a(0),b(0),c(0),d(0) 
    A(const A & orig){ 
     *this = orig;  /* <----- is this "bad"? */ 
     c = int(); 
    } 
    public: 
    int a,b,c,d; 
}; 

A X; 
X.a = 123; 
X.b = 456; 
X.c = 789; 
X.d = 987; 

A Y(X); 

printf("X: %d %d %d %d\n",X.a,X.b,X.c,X.d); 
printf("Y: %d %d %d %d\n",Y.a,Y.b,Y.c,Y.d); 

Salida:

X: 123 456 789 987 
Y: 123 456 0 987 

Alternativa constructor de copia:

A(const A & orig):a(orig.a),b(orig.b),c(0),d(orig.d){} /* <-- is this "better"? */ 
+2

ver: http://stackoverflow.com/questions/1457842/is-this-good-code-copy-ctor-operator – sth

Respuesta

4

Con su versión del constructor de copia son los miembros primero default-construida y luego asignado.
Con tipos integrales, esto no importa, pero si tuviera miembros no triviales como std::string s esto no es necesario.

Por lo tanto, sí, en general, su constructor de copia alternativo es mejor, pero si solo tiene integrales como miembros realmente no importa.

13

Como señala brone, será mejor que implemente la asignación en términos de construcción de copias. Yo prefiero un idioma alternativo al suyo:

T& T::operator=(T t) { 
    swap(*this, t); 
    return *this; 
} 

que es un poco más corto, y can take advantage of some esoteric language features para mejorar el rendimiento. Como cualquier buena pieza de código C++, también tiene algunas sutilezas a tener en cuenta.

Primero, el parámetro t se pasa intencionalmente por valor, por lo que se llamará al constructor de copia (most of the time) y podemos modificarlo en nuestro corazón sin afectar el valor original. El uso de const T& no se compilaría, y T& provocaría un comportamiento sorprendente al modificar el valor asignado.

Esta técnica también requiere swap ser especializada para el tipo de una manera que no utiliza operador de asignación del tipo (como std::swap hace), o causará una recursión infinita. Tenga cuidado con cualquier using std::swap o using namespace std extraviado, ya que van a tirar std::swap en el alcance y causar problemas si no se especializó swap para . La resolución de sobrecarga y ADL asegurarán que se use la versión correcta de swap si la ha definido.

Hay un par de formas de definir swap para un tipo. El primer método utiliza una función swap miembro para hacer el trabajo real y tiene una especialización swap que los delegados a la misma, así:

class T { 
public: 
    // .... 
    void swap(T&) { ... } 
}; 

void swap(T& a, T& b) { a.swap(b); } 

Esto es bastante común en la librería estándar; std::vector, por ejemplo, ha implementado el intercambio de esta manera. Si tiene una función de miembro swap, puede llamar directamente desde el operador de asignación y evitar cualquier problema con la búsqueda de funciones.

Otra forma es declarar swap en función amigo y hacer que haga todo el trabajo:

class T { 
    // .... 
    friend void swap(T& a, T& b); 
}; 

void swap(T& a, T& b) { ... } 

Yo prefiero la segunda, como swap() por lo general no es una parte integral de la interfaz de la clase ; parece más apropiado como una función gratuita. Es una cuestión de gusto, sin embargo.

Tener un swap optimizado para un tipo es un método común para lograr algunos de los beneficios de las referencias de valor en C++ 0x, por lo que es una buena idea en general si la clase puede aprovecharlo y realmente necesita el actuación.

+3

¡Esto es potencialmente peligroso! Sin una especialización explícita, std :: swap requiere un operador de asignación de copia de trabajo y puede usarlo en su implementación. Esto haría que este operador = infinitamente recursivo. –

+1

@Charles - editado, gracias. Debería haberlo sabido, ya que cometí el mismo error una vez. –

+0

OK, pero alguien podría tener 'use std :: swap' o (gasp)' using namespace std' sin proporcionar una especialización. ¿No es simplemente más fácil establecer un requisito de que la clase debe proporcionar una función de intercambio explícita (miembro no estático, miembro estático, función libre o especialización estándar de intercambio de datos) que no utiliza directamente el operador de asignación? En el caso de objetos intercambiables, por lo general, proporciono una función de miembro 'Swap' solo para que se deletree de manera diferente y capte los errores de forgetting-to-defined-it. Implemento una función 'swap' libre y un operador de asignación de copia que ambos difieren a la función' Swap'. –

0

Lo llamaría mala forma, no porque asigna dos veces todos sus objetos, sino porque en mi experiencia a menudo es mala forma confiar en en el operador predeterminado de copia/constructor de copias para un conjunto específico de funcionalidades. Dado que estos no están en la fuente en cualquier lugar, es difícil decir que el comportamiento que desea depende de su comportamiento. Por ejemplo, ¿qué pasa si alguien en un año quiere agregar un vector de cuerdas a su clase? Ya no tiene los viejos tipos de datos antiguos, y sería muy difícil para un mantenedor saber que estaban rompiendo cosas.

Creo que, bueno como DRY, crear requisitos sutiles no especificados es mucho peor desde el punto de vista del mantenimiento. Incluso repetirlo, por malo que sea, es menos malo.

1

Personalmente, creo que el operador de asignación roto es asesino. Siempre digo que las personas deben leer la documentación y no hacer nada, pero les dice que no lo hagan, pero aun así es demasiado fácil escribir una tarea sin pensarlo, o usar una plantilla que requiera que el tipo sea asignable. Hay una razón para el modismo no copiable: si operator= no va a funcionar, es demasiado peligroso dejarlo accesible.

Si no recuerdo mal, C++ 0x le permitirá hacer esto:

private: 
    A &operator=(const A &) = default; 

Entonces, al menos es sólo la propia clase que puede utilizar el operador de asignación por defecto rota y había esperanza de que en este contexto restringido es más fácil tener cuidado.

0

Creo que la mejor manera de no implementar un constructor de copia si el comportamiento es trivial (en su caso parece no estar funcionando: al menos misiones y la copia deben tener una semántica similar, pero su código sugiere que esto no lo hará ser así, pero entonces supongo que es un ejemplo artificial). El código que se genera para usted no puede estar equivocado.

Si necesita para implementar esos métodos, lo más probable es que la clase podría hacer con un método de intercambio rápido y así poder reutilizar el constructor de copia para implementar el operador de asignación.

Si por alguna razón necesita proporcionar una copia superficial constructor por defecto, entonces C++ 0X tiene

X(const X&) = default; 

Pero yo no creo que es un modismo para la semántica extraños. En este caso, el uso de la asignación en lugar de la inicialización es barato (ya que dejar las entradas sin inicializar no cuesta nada), por lo que podría hacerlo así.

2

Básicamente, lo que dices es que tienes algunos miembros de tu clase que no contribuyen a la identidad de la clase. Tal como se encuentra actualmente, esto se expresa mediante el uso del operador de asignación para copiar los miembros de la clase y luego restablecer los miembros que no deberían copiarse. Esto te deja con un operador de asignación que es inconsistente con el constructor de copia.

Mucho mejor sería utilizar el modificador de copiar y cambiar, y expresar qué miembros no deberían copiarse en el constructor de copia. Todavía tiene un lugar donde se expresa el comportamiento de "no copiar este miembro", pero ahora su operador de asignación y el constructor de copia son consistentes.

class A 
{ 
public: 

    A() : a(), b(), c(), d() {} 

    A(const A& other) 
     : a(other.a) 
     , b(other.b) 
     , c() // c isn't copied! 
     , d(other.d) 

    A& operator=(const A& other) 
    { 
     A tmp(other); // doesn't copy other.c 
     swap(tmp); 
     return *this; 
    } 

    void Swap(A& other) 
    { 
     using std::swap; 
     swap(a, other.a); 
     swap(b, other.b); 
     swap(c, other.c); // see note 
     swap(d, other.d); 
    } 

private: 
    // ... 
}; 

Nota: en la función de miembro de swap, he cambiado el miembro de c. Para los fines del uso en el operador de asignación, esto preserva el comportamiento para que coincida con el del constructor de copia: reinicia el miembro c. Si deja la función swap como pública, o proporciona acceso a ella a través de una función gratuita swap, debe asegurarse de que este comportamiento sea adecuado para otros usos de swap.

+0

operador = debe tomar A por valor - Revisé mi respuesta para llamar a esto. Además, olvidaste devolver * esto. –

+1

@Jeff Hardy: Tendría que argumentar que es un problema de estilo. Tomar el argumento por valor tiene algunas ventajas, pero la copia es fácil de pasar por alto. Tomando una referencia constante hace que la copia sea más obvia. Tomar por valor puede permitir una optimización adicional en algunas situaciones, pero nunca lo he visto marcar una diferencia notable. –

Cuestiones relacionadas