From 5bce0b9e065947d17ce091d7098f29aa37df03de Mon Sep 17 00:00:00 2001
From: Oliver Hamlet <oliver@ortham.io>
Date: Sat, 19 Feb 2022 13:23:22 +0000
Subject: [PATCH] Move ToSimpleMessage out of Message

It doesn't need access to any private members.
---
 include/loot/metadata/message.h               | 21 ++++++------
 src/api/metadata/message.cpp                  | 34 +++++++++----------
 .../api/internals/metadata/message_test.h     | 12 +++----
 3 files changed, 34 insertions(+), 33 deletions(-)

diff --git a/include/loot/metadata/message.h b/include/loot/metadata/message.h
index bc5cc95a..37e245ff 100644
--- a/include/loot/metadata/message.h
+++ b/include/loot/metadata/message.h
@@ -96,16 +96,6 @@ public:
    */
   LOOT_API std::vector<MessageContent> GetContent() const;
 
-  /**
-   * Get the message as a SimpleMessage given a language.
-   * @param  language
-   *         The preferred language for the message content.
-   * @return A SimpleMessage object for the preferred language, or for English
-   *         if message text is not available for the given language.
-   */
-  LOOT_API std::optional<SimpleMessage> ToSimpleMessage(
-      const std::string& language) const;
-
 private:
   MessageType type_{MessageType::say};
   std::vector<MessageContent> content_;
@@ -153,6 +143,17 @@ LOOT_API bool operator<=(const Message& lhs, const Message& rhs);
  *          Message object, false otherwise.
  */
 LOOT_API bool operator>=(const Message& lhs, const Message& rhs);
+
+/**
+ * Get the message as a SimpleMessage given a language.
+ * @param  language
+ *         The preferred language for the message content.
+ * @return A SimpleMessage object for the preferred language, or for English
+ *         if message text is not available for the given language.
+ */
+LOOT_API std::optional<SimpleMessage> ToSimpleMessage(
+    const Message& message,
+    const std::string& language);
 }
 
 #endif
diff --git a/src/api/metadata/message.cpp b/src/api/metadata/message.cpp
index d4048703..981dd301 100644
--- a/src/api/metadata/message.cpp
+++ b/src/api/metadata/message.cpp
@@ -62,23 +62,6 @@ MessageType Message::GetType() const { return type_; }
 
 std::vector<MessageContent> Message::GetContent() const { return content_; }
 
-std::optional<SimpleMessage> Message::ToSimpleMessage(
-    const std::string& language) const {
-  auto content = SelectMessageContent(content_, language);
-  if (!content.has_value()) {
-    return std::nullopt;
-  }
-
-  SimpleMessage simpleMessage;
-
-  simpleMessage.type = GetType();
-  simpleMessage.language = content.value().GetLanguage();
-  simpleMessage.text = content.value().GetText();
-  simpleMessage.condition = GetCondition();
-
-  return simpleMessage;
-}
-
 bool operator==(const Message& lhs, const Message& rhs) {
   return lhs.GetType() == rhs.GetType() &&
          lhs.GetCondition() == rhs.GetCondition() &&
@@ -114,4 +97,21 @@ bool operator>(const Message& lhs, const Message& rhs) { return rhs < lhs; }
 bool operator<=(const Message& lhs, const Message& rhs) { return !(lhs > rhs); }
 
 bool operator>=(const Message& lhs, const Message& rhs) { return !(lhs < rhs); }
+
+std::optional<SimpleMessage> ToSimpleMessage(const Message& message,
+                                             const std::string& language) {
+  auto content = SelectMessageContent(message.GetContent(), language);
+  if (!content.has_value()) {
+    return std::nullopt;
+  }
+
+  SimpleMessage simpleMessage;
+
+  simpleMessage.type = message.GetType();
+  simpleMessage.language = content.value().GetLanguage();
+  simpleMessage.text = content.value().GetText();
+  simpleMessage.condition = message.GetCondition();
+
+  return simpleMessage;
+}
 }
diff --git a/src/tests/api/internals/metadata/message_test.h b/src/tests/api/internals/metadata/message_test.h
index 286bd007..226465de 100644
--- a/src/tests/api/internals/metadata/message_test.h
+++ b/src/tests/api/internals/metadata/message_test.h
@@ -363,20 +363,20 @@ TEST_P(
   EXPECT_TRUE(message2 >= message1);
 }
 
-TEST_P(MessageTest, toSimpleMessageShouldSelectTextAndLanguageUsingGetContent) {
+TEST(ToSimpleMessage, shouldSelectTextAndLanguageUsingGetContent) {
   Message message(MessageType::warn,
-                  MessageContents({
-                      MessageContent("content1", german),
+                  std::vector<MessageContent>({
+                      MessageContent("content1", "de"),
                       MessageContent("content2"),
-                      MessageContent("content3", french),
+                      MessageContent("content3", "fr"),
                   }),
                   "condition1");
 
-  SimpleMessage simpleMessage = message.ToSimpleMessage(french).value();
+  SimpleMessage simpleMessage = ToSimpleMessage(message, "fr").value();
 
   EXPECT_EQ(MessageType::warn, simpleMessage.type);
   EXPECT_EQ("content3", simpleMessage.text);
-  EXPECT_EQ(french, simpleMessage.language);
+  EXPECT_EQ("fr", simpleMessage.language);
   EXPECT_EQ("condition1", simpleMessage.condition);
 }