Move overloaded symmetric operators out of classes

This follows the advice of C++ Core Guidelines C.161 and C.168.

The File equality and less than comparison operators have not been
moved because File::GetDisplayName() is not a trivial accessor.

include/loot/metadata/group.h

@@ -62,20 +62,6 @@ public:
                           const std::vector<std::string>& afterGroups = {},
                           const std::string& description = "");
 
-  /**
-   * Check if two Group objects are equal by comparing their names.
-   * @returns True if the objects' fields are equal, false otherwise.
-   */
-  LOOT_API bool operator==(const Group& rhs) const;
-
-  /**
-   * A less-than operator implemented with no semantics so that Group objects
-   * can be stored in sets.
-   * @returns True if this Group is less than the given Group, false
-   *          otherwise.
-   */
-  LOOT_API bool operator<(const Group& rhs) const;
-
   /**
    * Get the name of the group.
    * @return The group's name.
@@ -100,12 +86,26 @@ private:
   std::vector<std::string> afterGroups_;
 };
 
+/**
+ * Check if two Group objects are equal by comparing their names.
+ * @returns True if the objects' fields are equal, false otherwise.
+ */
+LOOT_API bool operator==(const Group& lhs, const Group& rhs);
+
 /**
  * Check if two Group objects are not equal.
  * @returns True if the Group objects are not equal, false otherwise.
  */
 LOOT_API bool operator!=(const Group& lhs, const Group& rhs);
 
+/**
+ * A less-than operator implemented with no semantics so that Group objects
+ * can be stored in sets.
+ * @returns True if the first Group is less than the second Group, false
+ *          otherwise.
+ */
+LOOT_API bool operator<(const Group& lhs, const Group& rhs);
+
 /**
  * Check if the first Group object is greater than the second Group
  * object.

include/loot/metadata/location.h

@@ -52,20 +52,6 @@ public:
   LOOT_API explicit Location(const std::string& url,
                              const std::string& name = "");
 
-  /**
-   * A less-than operator implemented with no semantics so that Location objects
-   * can be stored in sets.
-   * @returns True if this Location is less than the given Location, false
-   *          otherwise.
-   */
-  LOOT_API bool operator<(const Location& rhs) const;
-
-  /**
-   * Check if two Location objects are equal by comparing their fields.
-   * @returns True if the objects' fields are equal, false otherwise.
-   */
-  LOOT_API bool operator==(const Location& rhs) const;
-
   /**
    * Get the object's URL.
    * @return A URL string.
@@ -83,12 +69,26 @@ private:
   std::string name_;
 };
 
+/**
+ * Check if two Location objects are equal by comparing their fields.
+ * @returns True if the objects' fields are equal, false otherwise.
+ */
+LOOT_API bool operator==(const Location& lhs, const Location& rhs);
+
 /**
  * Check if two Location objects are not equal.
  * @returns True if the Location objects are not equal, false otherwise.
  */
 LOOT_API bool operator!=(const Location& lhs, const Location& rhs);
 
+/**
+ * A less-than operator implemented with no semantics so that Location objects
+ * can be stored in sets.
+ * @returns True if the first Location is less than the second Location, false
+ *          otherwise.
+ */
+LOOT_API bool operator<(const Location& lhs, const Location& rhs);
+
 /**
  * Check if the first Location object is greater than the second Location
  * object.

include/loot/metadata/message.h

@@ -84,20 +84,6 @@ public:
    */
   LOOT_API explicit Message(const SimpleMessage& message);
 
-  /**
-   * A less-than operator implemented with no semantics so that Message objects
-   * can be stored in sets.
-   * @returns Returns true if this Message is less than the given Message, and
-   *          false otherwise.
-   */
-  LOOT_API bool operator<(const Message& rhs) const;
-
-  /**
-   * Check if two Message objects are equal by comparing their fields.
-   * @returns True if the objects' fields are equal, false otherwise.
-   */
-  LOOT_API bool operator==(const Message& rhs) const;
-
   /**
    * Get the message type.
    * @return The message type.
@@ -125,12 +111,26 @@ private:
   std::vector<MessageContent> content_;
 };
 
+/**
+ * Check if two Message objects are equal by comparing their fields.
+ * @returns True if the objects' fields are equal, false otherwise.
+ */
+LOOT_API bool operator==(const Message& lhs, const Message& rhs);
+
 /**
  * Check if two Message objects are not equal.
  * @returns True if the Message objects are not equal, false otherwise.
  */
 LOOT_API bool operator!=(const Message& lhs, const Message& rhs);
 
+/**
+ * A less-than operator implemented with no semantics so that Message objects
+ * can be stored in sets.
+ * @returns Returns true if the first Message is less than the second Message,
+ *          and false otherwise.
+ */
+LOOT_API bool operator<(const Message& lhs, const Message& rhs);
+
 /**
  * Check if the first Message object is greater than the second Message object.
  * @returns True if the second Message object is less than the first Message

include/loot/metadata/message_content.h

@@ -72,31 +72,31 @@ public:
    */
   LOOT_API std::string GetLanguage() const;
 
-  /**
-   * A less-than operator implemented with no semantics so that MessageContent
-   * objects can be stored in sets.
-   * @returns True if this MessageContent is less than the given
-   *          MessageContent, false otherwise.
-   */
-  LOOT_API bool operator<(const MessageContent& rhs) const;
-
-  /**
-   * Check if two MessageContent objects are equal by comparing their fields.
-   * @returns True if the objects' fields are equal, false otherwise.
-   */
-  LOOT_API bool operator==(const MessageContent& rhs) const;
-
 private:
   std::string text_;
   std::string language_{DEFAULT_LANGUAGE};
 };
 
+/**
+ * Check if two MessageContent objects are equal by comparing their fields.
+ * @returns True if the objects' fields are equal, false otherwise.
+ */
+LOOT_API bool operator==(const MessageContent& lhs, const MessageContent& rhs);
+
 /**
  * Check if two MessageContent objects are not equal.
  * @returns True if the MessageContent objects are not equal, false otherwise.
  */
 LOOT_API bool operator!=(const MessageContent& lhs, const MessageContent& rhs);
 
+/**
+ * A less-than operator implemented with no semantics so that MessageContent
+ * objects can be stored in sets.
+ * @returns True if the first MessageContent is less than the second
+ *          MessageContent, false otherwise.
+ */
+LOOT_API bool operator<(const MessageContent& lhs, const MessageContent& rhs);
+
 /**
  * Check if the first MessageContent object is greater than the second
  * MessageContent object.

include/loot/metadata/plugin_cleaning_data.h

@@ -83,20 +83,6 @@ public:
       unsigned int itm,
       unsigned int ref,
       unsigned int nav);
-  /**
-   * A less-than operator implemented with no semantics so that
-   * PluginCleaningData objects can be stored in sets.
-   * @returns True if this PluginCleaningData is less than the given
-   *          PluginCleaningData, false otherwise.
-   */
-  LOOT_API bool operator<(const PluginCleaningData& rhs) const;
-
-  /**
-   * Check if two PluginCleaningData objects are equal by comparing their
-   * fields.
-   * @returns True if the objects' fields are equal, false otherwise.
-   */
-  LOOT_API bool operator==(const PluginCleaningData& rhs) const;
 
   /**
    * Get the CRC that identifies the plugin that the cleaning data is for.
@@ -147,6 +133,14 @@ private:
   std::vector<MessageContent> detail_;
 };
 
+/**
+ * Check if two PluginCleaningData objects are equal by comparing their
+ * fields.
+ * @returns True if the objects' fields are equal, false otherwise.
+ */
+LOOT_API bool operator==(const PluginCleaningData& lhs,
+                         const PluginCleaningData& rhs);
+
 /**
  * Check if two MessageContent objects are not equal.
  * @returns True if the MessageContent objects are not equal, false otherwise.
@@ -154,6 +148,15 @@ private:
 LOOT_API bool operator!=(const PluginCleaningData& lhs,
                          const PluginCleaningData& rhs);
 
+/**
+ * A less-than operator implemented with no semantics so that
+ * PluginCleaningData objects can be stored in sets.
+ * @returns True if the first PluginCleaningData is less than the second
+ *          PluginCleaningData, false otherwise.
+ */
+LOOT_API bool operator<(const PluginCleaningData& lhs,
+                        const PluginCleaningData& rhs);
+
 /**
  * Check if the first PluginCleaningData object is greater than the second
  * PluginCleaningData object.

include/loot/metadata/tag.h

@@ -57,19 +57,6 @@ public:
                         const bool isAddition = true,
                         const std::string& condition = "");
 
-  /**
-   * A less-than operator implemented with no semantics so that Tag objects
-   * can be stored in sets.
-   * @returns True if this Tag is less than the given Tag, false otherwise.
-   */
-  LOOT_API bool operator<(const Tag& rhs) const;
-
-  /**
-   * Check if two Tag objects are equal.
-   * @returns True if the objects' fields are equal, false otherwise.
-   */
-  LOOT_API bool operator==(const Tag& rhs) const;
-
   /**
    * Check if the tag should be added.
    * @return True if the tag should be added, false if it should be removed.
@@ -87,12 +74,25 @@ private:
   bool addTag_{true};
 };
 
+/**
+ * Check if two Tag objects are equal.
+ * @returns True if the objects' fields are equal, false otherwise.
+ */
+LOOT_API bool operator==(const Tag& lhs, const Tag& rhs);
+
 /**
  * Check if two Tag objects are not equal.
  * @returns True if the Tag objects are not equal, false otherwise.
  */
 LOOT_API bool operator!=(const Tag& lhs, const Tag& rhs);
 
+/**
+ * A less-than operator implemented with no semantics so that Tag objects
+ * can be stored in sets.
+ * @returns True if the first Tag is less than the second Tag, false otherwise.
+ */
+LOOT_API bool operator<(const Tag& lhs, const Tag& rhs);
+
 /**
  * Check if the first Tag object is greater than the second Tag object.
  * @returns True if the second Tag object is less than the first Tag object,

src/api/metadata/group.cpp

@@ -32,39 +32,40 @@ Group::Group(const std::string& name,
              const std::string& description) :
     name_(name), description_(description), afterGroups_(afterGroups) {}
 
-bool Group::operator==(const Group& rhs) const {
-  return name_ == rhs.name_ && description_ == rhs.description_ &&
-         afterGroups_ == rhs.afterGroups_;
-}
-
-bool Group::operator<(const Group& rhs) const {
-  if (name_ < rhs.name_) {
-    return true;
-  }
-
-  if (rhs.name_ < name_) {
-    return false;
-  }
-
-  if (description_ < rhs.description_) {
-    return true;
-  }
-
-  if (rhs.description_ < description_) {
-    return false;
-  }
-
-  return afterGroups_ < rhs.afterGroups_;
-}
-
 std::string Group::GetName() const { return name_; }
 
 std::string Group::GetDescription() const { return description_; }
 
 std::vector<std::string> Group::GetAfterGroups() const { return afterGroups_; }
 
+bool operator==(const Group& lhs, const Group& rhs) {
+  return lhs.GetName() == rhs.GetName() &&
+         lhs.GetDescription() == rhs.GetDescription() &&
+         lhs.GetAfterGroups() == rhs.GetAfterGroups();
+}
+
 bool operator!=(const Group& lhs, const Group& rhs) { return !(lhs == rhs); }
 
+bool operator<(const Group& lhs, const Group& rhs) {
+  if (lhs.GetName() < rhs.GetName()) {
+    return true;
+  }
+
+  if (rhs.GetName() < lhs.GetName()) {
+    return false;
+  }
+
+  if (lhs.GetDescription() < rhs.GetDescription()) {
+    return true;
+  }
+
+  if (rhs.GetDescription() < lhs.GetDescription()) {
+    return false;
+  }
+
+  return lhs.GetAfterGroups() < rhs.GetAfterGroups();
+}
+
 bool operator>(const Group& lhs, const Group& rhs) { return rhs < lhs; }
 
 bool operator<=(const Group& lhs, const Group& rhs) { return !(lhs > rhs); }

src/api/metadata/location.cpp

@@ -28,30 +28,30 @@ namespace loot {
 Location::Location(const std::string& url, const std::string& name) :
     url_(url), name_(name) {}
 
-bool Location::operator<(const Location& rhs) const {
-  if (url_ < rhs.url_) {
-    return true;
-  }
-
-  if (rhs.url_ < url_) {
-    return false;
-  }
-
-  return name_ < rhs.name_;
-}
-
-bool Location::operator==(const Location& rhs) const {
-  return url_ == rhs.url_ && name_ == rhs.name_;
-}
-
 std::string Location::GetURL() const { return url_; }
 
 std::string Location::GetName() const { return name_; }
 
+bool operator==(const Location& lhs, const Location& rhs) {
+  return lhs.GetURL() == rhs.GetURL() && lhs.GetName() == rhs.GetName();
+}
+
 bool operator!=(const Location& lhs, const Location& rhs) {
   return !(lhs == rhs);
 }
 
+bool operator<(const Location& lhs, const Location& rhs) {
+  if (lhs.GetURL() < rhs.GetURL()) {
+    return true;
+  }
+
+  if (rhs.GetURL() < lhs.GetURL()) {
+    return false;
+  }
+
+  return lhs.GetName() < rhs.GetName();
+}
+
 bool operator>(const Location& lhs, const Location& rhs) { return rhs < lhs; }
 
 bool operator<=(const Location& lhs, const Location& rhs) {

src/api/metadata/message.cpp

@@ -58,31 +58,6 @@ Message::Message(const SimpleMessage& message) :
     type_(message.type),
     content_({MessageContent(message.text, message.language)}) {}
 
-bool Message::operator<(const Message& rhs) const {
-  if (type_ < rhs.type_) {
-    return true;
-  }
-
-  if (rhs.type_ < type_) {
-    return false;
-  }
-
-  if (GetCondition() < rhs.GetCondition()) {
-    return true;
-  }
-
-  if (rhs.GetCondition() < GetCondition()) {
-    return false;
-  }
-
-  return content_ < rhs.GetContent();
-}
-
-bool Message::operator==(const Message& rhs) const {
-  return type_ == rhs.type_ && GetCondition() == rhs.GetCondition() &&
-         content_ == rhs.GetContent();
-}
-
 MessageType Message::GetType() const { return type_; }
 
 std::vector<MessageContent> Message::GetContent() const { return content_; }
@@ -104,10 +79,36 @@ std::optional<SimpleMessage> Message::ToSimpleMessage(
   return simpleMessage;
 }
 
+bool operator==(const Message& lhs, const Message& rhs) {
+  return lhs.GetType() == rhs.GetType() &&
+         lhs.GetCondition() == rhs.GetCondition() &&
+         lhs.GetContent() == rhs.GetContent();
+}
+
 bool operator!=(const Message& lhs, const Message& rhs) {
   return !(lhs == rhs);
 }
 
+bool operator<(const Message& lhs, const Message& rhs) {
+  if (lhs.GetType() < rhs.GetType()) {
+    return true;
+  }
+
+  if (rhs.GetType() < lhs.GetType()) {
+    return false;
+  }
+
+  if (lhs.GetCondition() < rhs.GetCondition()) {
+    return true;
+  }
+
+  if (rhs.GetCondition() < lhs.GetCondition()) {
+    return false;
+  }
+
+  return lhs.GetContent() < rhs.GetContent();
+}
+
 bool operator>(const Message& lhs, const Message& rhs) { return rhs < lhs; }
 
 bool operator<=(const Message& lhs, const Message& rhs) { return !(lhs > rhs); }

src/api/metadata/message_content.cpp

@@ -36,26 +36,27 @@ std::string MessageContent::GetText() const { return text_; }
 
 std::string MessageContent::GetLanguage() const { return language_; }
 
-bool MessageContent::operator<(const MessageContent& rhs) const {
-  if (text_ < rhs.text_) {
-    return true;
-  }
-
-  if (rhs.text_ < text_) {
-    return false;
-  }
-
-  return language_ < rhs.language_;
-}
-
-bool MessageContent::operator==(const MessageContent& rhs) const {
-  return text_ == rhs.text_ && language_ == rhs.language_;
+bool operator==(const MessageContent& lhs, const MessageContent& rhs) {
+  return lhs.GetText() == rhs.GetText() &&
+         lhs.GetLanguage() == rhs.GetLanguage();
 }
 
 bool operator!=(const MessageContent& lhs, const MessageContent& rhs) {
   return !(lhs == rhs);
 }
 
+bool operator<(const MessageContent& lhs, const MessageContent& rhs) {
+  if (lhs.GetText() < rhs.GetText()) {
+    return true;
+  }
+
+  if (rhs.GetText() < lhs.GetText()) {
+    return false;
+  }
+
+  return lhs.GetLanguage() < rhs.GetLanguage();
+}
+
 bool operator>(const MessageContent& lhs, const MessageContent& rhs) {
   return rhs < lhs;
 }

src/api/metadata/plugin_cleaning_data.cpp

@@ -47,56 +47,6 @@ PluginCleaningData::PluginCleaningData(
     utility_(utility),
     detail_(detail) {}
 
-bool PluginCleaningData::operator<(const PluginCleaningData& rhs) const {
-  if (crc_ < rhs.crc_) {
-    return true;
-  }
-
-  if (rhs.crc_ < crc_) {
-    return false;
-  }
-
-  if (utility_ < rhs.utility_) {
-    return true;
-  }
-
-  if (rhs.utility_ < utility_) {
-    return false;
-  }
-
-  if (itm_ < rhs.itm_) {
-    return true;
-  }
-
-  if (rhs.itm_ < itm_) {
-    return false;
-  }
-
-  if (ref_ < rhs.ref_) {
-    return true;
-  }
-
-  if (rhs.ref_ < ref_) {
-    return false;
-  }
-
-  if (nav_ < rhs.nav_) {
-    return true;
-  }
-
-  if (rhs.nav_ < nav_) {
-    return false;
-  }
-
-  return detail_ < rhs.detail_;
-}
-
-bool PluginCleaningData::operator==(const PluginCleaningData& rhs) const {
-  return crc_ == rhs.crc_ && utility_ == rhs.utility_ &&
-         detail_ == rhs.detail_ && itm_ == rhs.itm_ && ref_ == rhs.ref_ &&
-         nav_ == rhs.nav_;
-}
-
 uint32_t PluginCleaningData::GetCRC() const { return crc_; }
 
 unsigned int PluginCleaningData::GetITMCount() const { return itm_; }
@@ -113,10 +63,63 @@ std::vector<MessageContent> PluginCleaningData::GetDetail() const {
   return detail_;
 }
 
+bool operator==(const PluginCleaningData& lhs, const PluginCleaningData& rhs) {
+  return lhs.GetCRC() == rhs.GetCRC() &&
+         lhs.GetITMCount() == rhs.GetITMCount() &&
+         lhs.GetDeletedReferenceCount() == rhs.GetDeletedReferenceCount() &&
+         lhs.GetDeletedNavmeshCount() == rhs.GetDeletedNavmeshCount() &&
+         lhs.GetCleaningUtility() == rhs.GetCleaningUtility() &&
+         lhs.GetDetail() == rhs.GetDetail();
+}
+
 bool operator!=(const PluginCleaningData& lhs, const PluginCleaningData& rhs) {
   return !(lhs == rhs);
 }
 
+bool operator<(const PluginCleaningData& lhs, const PluginCleaningData& rhs) {
+  if (lhs.GetCRC() < rhs.GetCRC()) {
+    return true;
+  }
+
+  if (rhs.GetCRC() < lhs.GetCRC()) {
+    return false;
+  }
+
+  if (lhs.GetCleaningUtility() < rhs.GetCleaningUtility()) {
+    return true;
+  }
+
+  if (rhs.GetCleaningUtility() < lhs.GetCleaningUtility()) {
+    return false;
+  }
+
+  if (lhs.GetITMCount() < rhs.GetITMCount()) {
+    return true;
+  }
+
+  if (rhs.GetITMCount() < lhs.GetITMCount()) {
+    return false;
+  }
+
+  if (lhs.GetDeletedReferenceCount() < rhs.GetDeletedReferenceCount()) {
+    return true;
+  }
+
+  if (rhs.GetDeletedReferenceCount() < lhs.GetDeletedReferenceCount()) {
+    return false;
+  }
+
+  if (lhs.GetDeletedNavmeshCount() < rhs.GetDeletedNavmeshCount()) {
+    return true;
+  }
+
+  if (rhs.GetDeletedNavmeshCount() < lhs.GetDeletedNavmeshCount()) {
+    return false;
+  }
+
+  return lhs.GetDetail() < rhs.GetDetail();
+}
+
 bool operator>(const PluginCleaningData& lhs, const PluginCleaningData& rhs) {
   return rhs < lhs;
 }

src/api/metadata/tag.cpp

@@ -32,33 +32,34 @@ Tag::Tag(const std::string& tag,
          const std::string& condition) :
     ConditionalMetadata(condition), name_(tag), addTag_(isAddition) {}
 
-bool Tag::operator<(const Tag& rhs) const {
-  if (addTag_ != rhs.addTag_) {
-    return addTag_ && !rhs.addTag_;
-  }
-
-  if (name_ < rhs.name_) {
-    return true;
-  }
-
-  if (rhs.name_ < name_) {
-    return false;
-  }
-
-  return GetCondition() < rhs.GetCondition();
-}
-
-bool Tag::operator==(const Tag& rhs) const {
-  return addTag_ == rhs.addTag_ && name_ == rhs.name_ &&
-         GetCondition() == rhs.GetCondition();
-}
-
 bool Tag::IsAddition() const { return addTag_; }
 
 std::string Tag::GetName() const { return name_; }
 
+bool operator==(const Tag& lhs, const Tag& rhs) {
+  return lhs.IsAddition() == rhs.IsAddition() &&
+         lhs.GetName() == rhs.GetName() &&
+         lhs.GetCondition() == rhs.GetCondition();
+}
+
 bool operator!=(const Tag& lhs, const Tag& rhs) { return !(lhs == rhs); }
 
+bool operator<(const Tag& lhs, const Tag& rhs) {
+  if (lhs.IsAddition() != rhs.IsAddition()) {
+    return lhs.IsAddition() && !rhs.IsAddition();
+  }
+
+  if (lhs.GetName() < rhs.GetName()) {
+    return true;
+  }
+
+  if (rhs.GetName() < lhs.GetName()) {
+    return false;
+  }
+
+  return lhs.GetCondition() < rhs.GetCondition();
+}
+
 bool operator>(const Tag& lhs, const Tag& rhs) { return rhs < lhs; }
 
 bool operator<=(const Tag& lhs, const Tag& rhs) { return !(lhs > rhs); }

src/api/sorting/plugin_graph.cpp

@@ -115,6 +115,10 @@ std::string describeEdgeType(EdgeType edgeType) {
   }
 }
 
+bool operator==(const GraphPath& lhs, const GraphPath& rhs) {
+  return lhs.from == rhs.from && lhs.to == rhs.to;
+}
+
 size_t PluginGraph::CountVertices() const {
   return boost::num_vertices(graph_);
 }

src/api/sorting/plugin_graph.h

@@ -53,14 +53,11 @@ typedef boost::associative_property_map<std::map<vertex_t, size_t>>
 std::string describeEdgeType(EdgeType edgeType);
 
 struct GraphPath {
-
-  bool operator==(const GraphPath& rhs) const {
-    return this->from == rhs.from && this->to == rhs.to;
-  }
-
   vertex_t from;
   vertex_t to;
 };
+
+bool operator==(const GraphPath& lhs, const GraphPath& rhs);
 }
 
 namespace std {