small changes in constructors

include/cif++/category.hpp

@@ -141,31 +141,39 @@ class category
 
 	/// \endcond
 
-	category() = default;                     ///< Default constructor
-	category(std::string_view name);          ///< Constructor taking a \a name
-	category(const category &rhs);            ///< Copy constructor
-	category(category &&rhs);                 ///< Move constructor
-	category &operator=(const category &rhs); ///< Copy assignement operator
-	category &operator=(category &&rhs);      ///< Move assignement operator
+	category() = default;            ///< Default constructor
+	category(std::string_view name); ///< Constructor taking a \a name
+	category(const category &rhs);   ///< Copy constructor
+
+	category(category &&rhs) noexcept ///< Move constructor
+	{
+		swap(*this, rhs);
+	}
+
+	category &operator=(category rhs) ///< assignement operator
+	{
+		swap(*this, rhs);
+		return *this;
+	}
 
 	/// @brief Destructor
 	/// @note Please note that the destructor is not virtual. It is assumed that
 	/// you will not derive from this class.
 	~category();
 
+	friend void swap(category &a, category &b) noexcept;
+
 	// --------------------------------------------------------------------
 
 	const std::string &name() const { return m_name; } ///< Returns the name of the category
 
-	[[deprecated("use key_items instead")]]
-	iset key_fields() const;                           ///< Returns the cif::iset of key item names. Retrieved from the @ref category_validator for this category
+	[[deprecated("use key_items instead")]] iset key_fields() const; ///< Returns the cif::iset of key item names. Retrieved from the @ref category_validator for this category
 
-	iset key_items() const;                           ///< Returns the cif::iset of key item names. Retrieved from the @ref category_validator for this category
+	iset key_items() const; ///< Returns the cif::iset of key item names. Retrieved from the @ref category_validator for this category
 
-	[[deprecated("use key_item_indices instead")]]
-	std::set<uint16_t> key_field_indices() const;      ///< Returns a set of indices for the key items.
+	[[deprecated("use key_item_indices instead")]] std::set<uint16_t> key_field_indices() const; ///< Returns a set of indices for the key items.
 
-	std::set<uint16_t> key_item_indices() const;      ///< Returns a set of indices for the key items.
+	std::set<uint16_t> key_item_indices() const; ///< Returns a set of indices for the key items.
 
 	/// @brief Set the validator for this category to @a v
 	/// @param v The category_validator to assign. A nullptr value is allowed.
@@ -1010,66 +1018,60 @@ class category
 
 	void update_value(const std::vector<row_handle> &rows, std::string_view item_name, std::string_view value)
 	{
-		update_value(rows, item_name, [value](std::string_view) { return value; });
+		update_value(rows, item_name, [value](std::string_view)
+			{ return value; });
 	}
 
 	// --------------------------------------------------------------------
 	// Naming used to be very inconsistent. For backward compatibility,
 	// the old function names are here as deprecated variants.
 
 	/// \brief Return the index number for \a column_name
-	[[deprecated("Use get_item_ix instead")]]
-	uint16_t get_column_ix(std::string_view column_name) const
+	[[deprecated("Use get_item_ix instead")]] uint16_t get_column_ix(std::string_view column_name) const
 	{
 		return get_item_ix(column_name);
 	}
 
 	/// @brief Return the name for column with index @a ix
 	/// @param ix The index number
 	/// @return The name of the column
-	[[deprecated("use get_item_name instead")]]
-	std::string_view get_column_name(uint16_t ix) const
+	[[deprecated("use get_item_name instead")]] std::string_view get_column_name(uint16_t ix) const
 	{
 		return get_item_name(ix);
 	}
 
 	/// @brief Make sure a item with name @a item_name is known and return its index number
 	/// @param item_name The name of the item
 	/// @return The index number of the item
-	[[deprecated("use add_item instead")]]
-	uint16_t add_column(std::string_view item_name)
+	[[deprecated("use add_item instead")]] uint16_t add_column(std::string_view item_name)
 	{
 		return add_item(item_name);
 	}
 
 	/** @brief Remove column name @a colum_name
 	 * @param column_name The column to be removed
 	 */
-	[[deprecated("use remove_item instead")]]
-	void remove_column(std::string_view column_name)
+	[[deprecated("use remove_item instead")]] void remove_column(std::string_view column_name)
 	{
 		remove_item(column_name);
 	}
 
 	/** @brief Rename column @a from_name to @a to_name */
-	[[deprecated("use rename_item instead")]]
-	void rename_column(std::string_view from_name, std::string_view to_name)
+	[[deprecated("use rename_item instead")]] void rename_column(std::string_view from_name, std::string_view to_name)
 	{
 		rename_item(from_name, to_name);
 	}
 
 	/// @brief Return whether a column with name @a name exists in this category
 	/// @param name The name of the column
 	/// @return True if the column exists
-	[[deprecated("use has_item instead")]]
-	bool has_column(std::string_view name) const
+	[[deprecated("use has_item instead")]] bool has_column(std::string_view name) const
 	{
 		return has_item(name);
 	}
 
 	/// @brief Return the cif::iset of columns in this category
-	[[deprecated("use get_items instead")]]
-	iset get_columns() const
+	[[deprecated("use get_items instead")]] iset get_columns() const
 	{
 		return get_items();
 	}
@@ -1125,7 +1127,7 @@ class category
 			{
 				item_validator = m_cat_validator->get_validator_for_item(item_name);
 				if (item_validator == nullptr)
-					m_validator->report_error( validation_error::item_not_allowed_in_category, m_name, item_name, false);
+					m_validator->report_error(validation_error::item_not_allowed_in_category, m_name, item_name, false);
 			}
 
 			m_items.emplace_back(item_name, item_validator);
@@ -1169,8 +1171,7 @@ class category
 
 	/// This function returns effectively the list of fully qualified item
 	/// names, that is category_name + '.' + item_name for each item
-	[[deprecated("use get_item_order instead")]]
-	std::vector<std::string> get_tag_order() const
+	[[deprecated("use get_item_order instead")]] std::vector<std::string> get_tag_order() const
 	{
 		return get_item_order();
 	}

include/cif++/datablock.hpp

@@ -61,12 +61,26 @@ class datablock : public std::list<category>
 
 	/** @cond */
 	datablock(const datablock &);
-	datablock(datablock &&) = default;
 
-	datablock &operator=(const datablock &);
-	datablock &operator=(datablock &&) = default;
+	datablock(datablock &&db) noexcept
+	{
+		swap_(*this, db);
+	}
+
+	datablock &operator=(datablock db)
+	{
+		swap_(*this, db);
+		return *this;
+	}
 	/** @endcond */
 
+	friend void swap_(datablock &a, datablock &b) noexcept
+	{
+		std::swap(a.m_name, b.m_name);
+		std::swap(a.m_validator, b.m_validator);
+		std::swap(static_cast<std::list<category>&>(a), static_cast<std::list<category>&>(b));
+	}
+
 	// --------------------------------------------------------------------
 
 	/**

include/cif++/item.hpp

@@ -288,19 +288,16 @@ struct item_value
 	}
 
 	/** @cond */
-	item_value(item_value &&rhs)
+	item_value(item_value &&rhs) noexcept
 		: m_length(std::exchange(rhs.m_length, 0))
 		, m_storage(std::exchange(rhs.m_storage, 0))
 	{
 	}
 
-	item_value &operator=(item_value &&rhs)
+	item_value &operator=(item_value &&rhs) noexcept
 	{
-		if (this != &rhs)
-		{
-			m_length = std::exchange(rhs.m_length, m_length);
-			m_storage = std::exchange(rhs.m_storage, m_storage);
-		}
+		std::swap(m_length, rhs.m_length);
+		std::swap(m_storage, rhs.m_storage);
 		return *this;
 	}
 

src/category.cpp

@@ -521,71 +521,18 @@ category::category(const category &rhs)
 		m_index = new category_index(*this);
 }
 
-category::category(category &&rhs)
-	: m_name(std::move(rhs.m_name))
-	, m_items(std::move(rhs.m_items))
-	, m_validator(rhs.m_validator)
-	, m_cat_validator(rhs.m_cat_validator)
-	, m_parent_links(std::move(rhs.m_parent_links))
-	, m_child_links(std::move(rhs.m_child_links))
-	, m_cascade(rhs.m_cascade)
-	, m_index(rhs.m_index)
-	, m_head(rhs.m_head)
-	, m_tail(rhs.m_tail)
-{
-	rhs.m_head = nullptr;
-	rhs.m_tail = nullptr;
-	rhs.m_index = nullptr;
-}
-
-category &category::operator=(const category &rhs)
-{
-	if (this != &rhs)
-	{
-		if (not empty())
-			clear();
-
-		m_name = rhs.m_name;
-		m_items = rhs.m_items;
-		m_cascade = rhs.m_cascade;
-
-		m_validator = nullptr;
-		m_cat_validator = nullptr;
-
-		delete m_index;
-		m_index = nullptr;
-
-		for (auto r = rhs.m_head; r != nullptr; r = r->m_next)
-			insert_impl(cend(), clone_row(*r));
-
-		m_validator = rhs.m_validator;
-		m_cat_validator = rhs.m_cat_validator;
-
-		if (m_cat_validator != nullptr and m_index == nullptr)
-			m_index = new category_index(*this);
-	}
-
-	return *this;
-}
-
-category &category::operator=(category &&rhs)
+void swap(category &a, category &b) noexcept
 {
-	if (this != &rhs)
-	{
-		m_name = std::move(rhs.m_name);
-		m_items = std::move(rhs.m_items);
-		m_cascade = rhs.m_cascade;
-		m_validator = rhs.m_validator;
-		m_cat_validator = rhs.m_cat_validator;
-		m_parent_links = rhs.m_parent_links;
-		m_child_links = rhs.m_child_links;
-
-		std::swap(m_index, rhs.m_index);
-		std::swap(m_head, rhs.m_head);
-		std::swap(m_tail, rhs.m_tail);
-	}
-
-	return *this;
+	std::swap(a.m_name, b.m_name);
+	std::swap(a.m_items, b.m_items);
+	std::swap(a.m_validator, b.m_validator);
+	std::swap(a.m_cat_validator, b.m_cat_validator);
+	std::swap(a.m_parent_links, b.m_parent_links);
+	std::swap(a.m_child_links, b.m_child_links);
+	std::swap(a.m_cascade, b.m_cascade);
+	std::swap(a.m_index, b.m_index);
+	std::swap(a.m_head, b.m_head);
+	std::swap(a.m_tail, b.m_tail);
 }
 
 category::~category()

src/datablock.cpp

@@ -38,21 +38,6 @@ datablock::datablock(const datablock &db)
 		cat.update_links(*this);
 }
 
-datablock &datablock::operator=(const datablock &db)
-{
-	if (this != &db)
-	{
-		std::list<category>::operator=(db);
-		m_name = db.m_name;
-		m_validator = db.m_validator;
-
-		for (auto &cat : *this)
-			cat.update_links(*this);
-	}
-
-	return *this;
-}
-
 void datablock::set_validator(const validator *v)
 {
 	m_validator = v;

src/pdb/pdb2cif.cpp

@@ -6413,11 +6413,11 @@ file read(std::istream &is)
 			{
 				std::throw_with_nested(std::runtime_error("Since the file did not start with a valid PDB HEADER line mmCIF was assumed, but that failed."));
 			}
-
-			// Since we're using the cif::pdb way of reading the file, the data may need
-			// reconstruction
-			reconstruct_pdbx(result);
 		}
+
+		// Since we're using the cif::pdb way of reading the file, the data may need
+		// reconstruction
+		reconstruct_pdbx(result);
 	}
 
 	// Must be a PDB like file, right?