Search code examples
c++copy-constructorinitializer-list

C++ Copy Constructors: must I spell out all member variables in the initializer list?


I have some pretty complicated objects. They contain member variables of other objects. I understand the beauty of copy constructors cascading such that the default copy constructor can often work. But, the situation that may most often break the default copy constructor (the object contains some member variables which are pointers to its other member variables) still applies to a lot of what I've built. Here's an example of one of my objects, its constructor, and the copy constructor I've written:

struct PhaseSpace {
  PhaseSpace();

private:
  std::string file_name;              ///< Name of the file from which these coordinates (and
                                      ///<   perhaps velocities) derived.  Empty string indicates
                                      ///<   no file.
  int atom_count;                     ///< The number of atoms in the system
  UnitCellType unit_cell;             ///< The type of unit cell
  Hybrid<double> x_coordinates;       ///< Cartesian X coordinates of all particles
  Hybrid<double> y_coordinates;       ///< Cartesian Y coordinates of all particles
  Hybrid<double> z_coordinates;       ///< Cartesian Z coordinates of all particles
  Hybrid<double> box_space_transform; ///< Matrix to transform coordinates into box space (3 x 3)
  Hybrid<double> inverse_transform;   ///< Matrix to transform coordinates into real space (3 x 3)
  Hybrid<double> box_dimensions;      ///< Three lengths and three angles defining the box (lengths
                                      ///<   are given in Angstroms, angles in radians)
  Hybrid<double> x_velocities;        ///< Cartesian X velocities of all particles
  Hybrid<double> y_velocities;        ///< Cartesian Y velocities of all particles
  Hybrid<double> z_velocities;        ///< Cartesian Z velocities of all particles
  Hybrid<double> x_forces;            ///< Cartesian X forces acting on all particles
  Hybrid<double> y_forces;            ///< Cartesian Y forces acting on all particles
  Hybrid<double> z_forces;            ///< Cartesian Z forces acting on all particles
  Hybrid<double> x_prior_coordinates; ///< Previous step Cartesian X coordinates of all particles
  Hybrid<double> y_prior_coordinates; ///< Previous step Cartesian Y coordinates of all particles
  Hybrid<double> z_prior_coordinates; ///< Previous step Cartesian Z coordinates of all particles

  /// All of the above Hybrid objects are pointers into this single large array, segmented to hold
  /// each type of information with zero-padding to accommodate the HPC warp size.
  Hybrid<double> storage;

  /// \brief Allocate space for the object, based on a known number of atoms
  void allocate();
};

//-------------------------------------------------------------------------------------------------
PhaseSpace::PhaseSpace() :
    file_name{std::string("")},
    atom_count{0},
    unit_cell{UnitCellType::NONE},
    x_coordinates{HybridKind::POINTER, "x_coordinates"},
    y_coordinates{HybridKind::POINTER, "y_coordinates"},
    z_coordinates{HybridKind::POINTER, "z_coordinates"},
    box_space_transform{HybridKind::POINTER, "box_transform"},
    inverse_transform{HybridKind::POINTER, "inv_transform"},
    box_dimensions{HybridKind::POINTER, "box_dimensions"},
    x_velocities{HybridKind::POINTER, "x_velocities"},
    y_velocities{HybridKind::POINTER, "y_velocities"},
    z_velocities{HybridKind::POINTER, "z_velocities"},
    x_forces{HybridKind::POINTER, "x_forces"},
    y_forces{HybridKind::POINTER, "y_forces"},
    z_forces{HybridKind::POINTER, "z_forces"},
    x_prior_coordinates{HybridKind::POINTER, "x_prior_coords"},
    y_prior_coordinates{HybridKind::POINTER, "y_prior_coords"},
    z_prior_coordinates{HybridKind::POINTER, "z_prior_coords"},
    storage{HybridKind::ARRAY, "phase_space_data"}
{}

//-------------------------------------------------------------------------------------------------
PhaseSpace::PhaseSpace(const PhaseSpace &original) :
    file_name{original.file_name},
    atom_count{original.atom_count},
    unit_cell{original.unit_cell},
    x_coordinates{original.x_coordinates},
    y_coordinates{original.y_coordinates},
    z_coordinates{original.z_coordinates},
    box_space_transform{original.box_space_transform},
    inverse_transform{original.inverse_transform},
    box_dimensions{original.box_dimensions},
    x_velocities{original.x_velocities},
    y_velocities{original.y_velocities},
    z_velocities{original.z_velocities},
    x_forces{original.x_forces},
    y_forces{original.y_forces},
    z_forces{original.z_forces},
    x_prior_coordinates{original.x_prior_coordinates},
    y_prior_coordinates{original.y_prior_coordinates},
    z_prior_coordinates{original.z_prior_coordinates},
    storage{original.storage}
{
  // Set the POINTER-kind Hybrids in the new object appropriately.  Even the resize() operation
  // inherent to "allocate" will pass by with little more than a check that the length of the data
  // storage array is already what it should be.
  allocate();
}

EDIT: here's the allocate() member function if it helps explain anything... with the storage array already having been allocated to the same length as the original and given a deep copy by the Hybrid object type's own copy-assignment constructor, all that remains is to set this PhaseSpace object's own POINTER-kind Hybrid objects to the appropriate locations in the ARRAY-kind Hybrid object storage.

//-------------------------------------------------------------------------------------------------
void PhaseSpace::allocate() {
  const int padded_atom_count  = roundUp(atom_count, warp_size_int);
  const int padded_matrix_size = roundUp(9, warp_size_int);
  storage.resize((15 * padded_atom_count) + (3 * padded_matrix_size));
  x_coordinates.setPointer(&storage,                            0, atom_count);
  y_coordinates.setPointer(&storage,            padded_atom_count, atom_count);
  z_coordinates.setPointer(&storage,        2 * padded_atom_count, atom_count);
  box_space_transform.setPointer(&storage,  3 * padded_atom_count, 9);
  inverse_transform.setPointer(&storage,   (3 * padded_atom_count) +      padded_matrix_size, 9);
  box_dimensions.setPointer(&storage,      (3 * padded_atom_count) + (2 * padded_matrix_size), 6);
  const int thus_far = (3 * padded_atom_count) + (3 * padded_matrix_size);
  x_velocities.setPointer(&storage,        thus_far,                           atom_count);
  y_velocities.setPointer(&storage,        thus_far +      padded_atom_count,  atom_count);
  z_velocities.setPointer(&storage,        thus_far + (2 * padded_atom_count), atom_count);
  x_forces.setPointer(&storage,            thus_far + (3 * padded_atom_count), atom_count);
  y_forces.setPointer(&storage,            thus_far + (4 * padded_atom_count), atom_count);
  z_forces.setPointer(&storage,            thus_far + (5 * padded_atom_count), atom_count);
  x_prior_coordinates.setPointer(&storage, thus_far + (6 * padded_atom_count), atom_count);
  y_prior_coordinates.setPointer(&storage, thus_far + (7 * padded_atom_count), atom_count);
  z_prior_coordinates.setPointer(&storage, thus_far + (8 * padded_atom_count), atom_count);
}

The comments are from the actual code, although I have (perhaps obviously) omitted a great deal of the details in the original struct definition. My question is whether the copy constructor needs such a detailed initializer list, or if there is a more elegant (and less error-prone) shorthand for making a custom copy constructor like this. Once I have an answer to this issue, I've got some much bigger objects with the same situation.

Cheers!


Solution

  • C++ Copy Constructors: must I spell out all member variables in the initializer list?

    Yes, if you write a user defined copy constructor, then you must write an initialiser for every sub object - unless you wish to default initialise them, in which case you don't need any initialiser - or if you can use a default member initialiser.

    the object contains some member variables which are pointers to its other member variables)

    This is a design that should be avoided when possible. Not only does this force you to define custom copy and move assignment operators and constructors, but it is often unnecessarily inefficient.

    But, in case that is necessary for some reason - or custom special member functions are needed for any other reason - you can achieve clean code by combining the normally copying parts into a separate dummy class. That way the the user defined constructor has only one sub object to initialise.

    Like this:

    struct just_data {
        // many members... details don't matter
        R just_data_1;
        S just_data_2;
        T just_data_3;
        U just_data_4;
        V just_data_5;
    };
    
    struct fancy_copying : just_data
    {
        fancy_copying(const fancy_copying& other)
            : just_data(other.just_data)
        {
            do_the_fancy();
        }
    };