Initialise load order handler on construction

And use a std::unique_ptr to avoid hitting the rule of five.

src/api/game/game.cpp

@@ -59,6 +59,7 @@ Game::Game(const GameType gameType,
            const std::filesystem::path& localDataPath) :
     type_(gameType),
     gamePath_(gamePath),
+    loadOrderHandler_(type_, gamePath_, localDataPath),
     conditionEvaluator_(
         std::make_shared<ConditionEvaluator>(Type(), DataPath())),
     database_(ApiDatabase(conditionEvaluator_)) {
@@ -68,8 +69,6 @@ Game::Game(const GameType gameType,
                  (int)type_,
                  gamePath_.u8string());
   }
-
-  loadOrderHandler_.Init(type_, gamePath_, localDataPath);
 }
 
 GameType Game::Type() const { return type_; }

src/api/game/load_order_handler.cpp

@@ -55,28 +55,13 @@ unsigned int mapGameId(GameType gameType) {
   }
 }
 
-LoadOrderHandler::LoadOrderHandler() : gh_(nullptr) {}
-
-LoadOrderHandler::LoadOrderHandler(LoadOrderHandler&& other) : gh_(other.gh_) {
-  other.gh_ = nullptr;
-}
-
-LoadOrderHandler::~LoadOrderHandler() { lo_destroy_handle(gh_); }
-
-LoadOrderHandler& LoadOrderHandler::operator=(LoadOrderHandler&& other) {
-  if (&other != this) {
-    lo_destroy_handle(gh_);
-
-    gh_ = other.gh_;
-    other.gh_ = nullptr;
-  }
-
-  return *this;
-}
-
-void LoadOrderHandler::Init(const GameType& gameType,
-                            const std::filesystem::path& gamePath,
-                            const std::filesystem::path& gameLocalAppData) {
+LoadOrderHandler::LoadOrderHandler(
+    const GameType& gameType,
+    const std::filesystem::path& gamePath,
+    const std::filesystem::path& gameLocalAppData) :
+    gh_(std::unique_ptr<std::remove_pointer<lo_game_handle>::type,
+                        decltype(&lo_destroy_handle)>(nullptr,
+                                                      lo_destroy_handle)) {
   if (gamePath.empty()) {
     throw std::invalid_argument("Game path is not initialised.");
   }
@@ -86,18 +71,18 @@ void LoadOrderHandler::Init(const GameType& gameType,
   if (!tempPathString.empty())
     gameLocalDataPath = tempPathString.c_str();
 
-  // If the handle has already been initialised, close it and open another.
-  if (gh_ != nullptr) {
-    lo_destroy_handle(gh_);
-    gh_ = nullptr;
-  }
+  lo_game_handle handle = nullptr;
 
-  int ret = lo_create_handle(&gh_,
+  int ret = lo_create_handle(&handle,
                              mapGameId(gameType),
                              gamePath.u8string().c_str(),
                              gameLocalDataPath);
 
   HandleError("create a game handle", ret);
+
+  gh_ =
+      std::unique_ptr<std::remove_pointer<lo_game_handle>::type,
+                      decltype(&lo_destroy_handle)>(handle, lo_destroy_handle);
 }
 
 void LoadOrderHandler::LoadCurrentState() {
@@ -106,7 +91,7 @@ void LoadOrderHandler::LoadCurrentState() {
     logger->info("Loading the current load order state.");
   }
 
-  const unsigned int ret = lo_load_current_state(gh_);
+  const unsigned int ret = lo_load_current_state(gh_.get());
 
   HandleError("load the current load order state", ret);
 }
@@ -119,7 +104,7 @@ bool LoadOrderHandler::IsPluginActive(const std::string& pluginName) const {
 
   bool result = false;
   const unsigned int ret =
-      lo_get_plugin_active(gh_, pluginName.c_str(), &result);
+      lo_get_plugin_active(gh_.get(), pluginName.c_str(), &result);
 
   HandleError("check if a plugin is active", ret);
 
@@ -135,7 +120,8 @@ std::vector<std::string> LoadOrderHandler::GetLoadOrder() const {
   char** pluginArr = nullptr;
   size_t pluginArrSize = 0;
 
-  const unsigned int ret = lo_get_load_order(gh_, &pluginArr, &pluginArrSize);
+  const unsigned int ret =
+      lo_get_load_order(gh_.get(), &pluginArr, &pluginArrSize);
 
   HandleError("get the load order", ret);
 
@@ -156,7 +142,7 @@ std::vector<std::string> LoadOrderHandler::GetActivePlugins() const {
   size_t pluginArrSize = 0;
 
   const unsigned int ret =
-      lo_get_active_plugins(gh_, &pluginArr, &pluginArrSize);
+      lo_get_active_plugins(gh_.get(), &pluginArr, &pluginArrSize);
 
   HandleError("get active plugins", ret);
 
@@ -177,7 +163,7 @@ std::vector<std::string> LoadOrderHandler::GetImplicitlyActivePlugins() const {
   size_t pluginArrSize = 0;
 
   const unsigned int ret =
-      lo_get_implicitly_active_plugins(gh_, &pluginArr, &pluginArrSize);
+      lo_get_implicitly_active_plugins(gh_.get(), &pluginArr, &pluginArrSize);
 
   HandleError("get implicitly active plugins", ret);
 
@@ -206,7 +192,7 @@ void LoadOrderHandler::SetLoadOrder(
   }
 
   const unsigned int ret =
-      lo_set_load_order(gh_, plugins.data(), plugins.size());
+      lo_set_load_order(gh_.get(), plugins.data(), plugins.size());
 
   HandleError("set the load order", ret);
 

src/api/game/load_order_handler.h

@@ -37,17 +37,9 @@
 namespace loot {
 class LoadOrderHandler {
 public:
-  explicit LoadOrderHandler();
-  LoadOrderHandler(const LoadOrderHandler&) = delete;
-  LoadOrderHandler(LoadOrderHandler&& other);
-  ~LoadOrderHandler();
-
-  LoadOrderHandler& operator=(const LoadOrderHandler&) = delete;
-  LoadOrderHandler& operator=(LoadOrderHandler&& other);
-
-  void Init(const GameType& game,
-            const std::filesystem::path& gamePath,
-            const std::filesystem::path& gameLocalAppData = "");
+  explicit LoadOrderHandler(const GameType& game,
+                            const std::filesystem::path& gamePath,
+                            const std::filesystem::path& gameLocalAppData = "");
 
   void LoadCurrentState();
 
@@ -64,7 +56,9 @@ public:
 private:
   void HandleError(const std::string& operation, unsigned int returnCode) const;
 
-  lo_game_handle gh_;
+  std::unique_ptr<std::remove_pointer<lo_game_handle>::type,
+                  decltype(&lo_destroy_handle)>
+      gh_;
 };
 }
 

src/tests/api/internals/game/load_order_handler_test.h

@@ -51,9 +51,8 @@ protected:
     }
   }
 
-  void initialiseHandler() {
-    ASSERT_NO_THROW(
-        loadOrderHandler_.Init(GetParam(), dataPath.parent_path(), localPath));
+  LoadOrderHandler createHandler() {
+    return LoadOrderHandler(GetParam(), dataPath.parent_path(), localPath);
   }
 
   std::vector<std::string> getImplicitlyActivePlugins() {
@@ -99,7 +98,6 @@ protected:
     return activePlugins;
   }
 
-  LoadOrderHandler loadOrderHandler_;
   std::vector<std::string> loadOrderToSet_;
 };
 
@@ -115,119 +113,94 @@ INSTANTIATE_TEST_SUITE_P(,
                                            GameType::fo4,
                                            GameType::tes5se));
 
-TEST_P(LoadOrderHandlerTest, initShouldThrowIfNoGamePathIsSet) {
-  EXPECT_THROW(loadOrderHandler_.Init(GetParam(), ""), std::invalid_argument);
-  EXPECT_THROW(loadOrderHandler_.Init(GetParam(), ""), std::invalid_argument);
-  EXPECT_THROW(loadOrderHandler_.Init(GetParam(), "", localPath),
+TEST_P(LoadOrderHandlerTest, constructorShouldThrowIfNoGamePathIsSet) {
+  EXPECT_THROW(LoadOrderHandler(GetParam(), ""), std::invalid_argument);
+  EXPECT_THROW(LoadOrderHandler(GetParam(), ""), std::invalid_argument);
+  EXPECT_THROW(LoadOrderHandler(GetParam(), "", localPath),
                std::invalid_argument);
-  EXPECT_THROW(loadOrderHandler_.Init(GetParam(), "", localPath),
+  EXPECT_THROW(LoadOrderHandler(GetParam(), "", localPath),
                std::invalid_argument);
 }
 
 #ifndef _WIN32
-TEST_P(LoadOrderHandlerTest, initShouldThrowOnLinuxIfNoLocalPathIsSet) {
-  EXPECT_THROW(loadOrderHandler_.Init(GetParam(), dataPath.parent_path()),
+TEST_P(LoadOrderHandlerTest, constructorShouldThrowOnLinuxIfNoLocalPathIsSet) {
+  EXPECT_THROW(LoadOrderHandler(GetParam(), dataPath.parent_path()),
                std::system_error);
 }
 #endif
 
 TEST_P(LoadOrderHandlerTest,
-       initShouldNotThrowIfAValidGameIdAndGamePathAndLocalPathAreSet) {
+       constructorShouldNotThrowIfAValidGameIdAndGamePathAndLocalPathAreSet) {
   EXPECT_NO_THROW(
-      loadOrderHandler_.Init(GetParam(), dataPath.parent_path(), localPath));
-}
-
-TEST_P(LoadOrderHandlerTest,
-       isPluginActiveShouldThrowIfTheHandlerHasNotBeenInitialised) {
-  EXPECT_THROW(loadOrderHandler_.IsPluginActive(masterFile), std::system_error);
+      LoadOrderHandler(GetParam(), dataPath.parent_path(), localPath));
 }
 
 TEST_P(LoadOrderHandlerTest,
        isPluginActiveShouldReturnFalseIfLoadOrderStateHasNotBeenLoaded) {
-  initialiseHandler();
+  auto loadOrderHandler = createHandler();
 
-  EXPECT_FALSE(loadOrderHandler_.IsPluginActive(masterFile));
-  EXPECT_FALSE(loadOrderHandler_.IsPluginActive(blankEsm));
-  EXPECT_FALSE(loadOrderHandler_.IsPluginActive(blankEsp));
+  EXPECT_FALSE(loadOrderHandler.IsPluginActive(masterFile));
+  EXPECT_FALSE(loadOrderHandler.IsPluginActive(blankEsm));
+  EXPECT_FALSE(loadOrderHandler.IsPluginActive(blankEsp));
 }
 
 TEST_P(LoadOrderHandlerTest,
        isPluginActiveShouldReturnCorrectPluginStatesAfterInitialisation) {
-  initialiseHandler();
-  loadOrderHandler_.LoadCurrentState();
+  auto loadOrderHandler = createHandler();
+  loadOrderHandler.LoadCurrentState();
 
-  EXPECT_TRUE(loadOrderHandler_.IsPluginActive(masterFile));
-  EXPECT_TRUE(loadOrderHandler_.IsPluginActive(blankEsm));
-  EXPECT_FALSE(loadOrderHandler_.IsPluginActive(blankEsp));
-}
-
-TEST_P(LoadOrderHandlerTest,
-       getLoadOrderShouldThrowIfTheHandlerHasNotBeenInitialised) {
-  EXPECT_THROW(loadOrderHandler_.GetLoadOrder(), std::system_error);
+  EXPECT_TRUE(loadOrderHandler.IsPluginActive(masterFile));
+  EXPECT_TRUE(loadOrderHandler.IsPluginActive(blankEsm));
+  EXPECT_FALSE(loadOrderHandler.IsPluginActive(blankEsp));
 }
 
 TEST_P(LoadOrderHandlerTest,
        getLoadOrderShouldReturnAnEmptyVectorIfStateHasNotBeenLoaded) {
-  initialiseHandler();
+  auto loadOrderHandler = createHandler();
 
-  EXPECT_TRUE(loadOrderHandler_.GetLoadOrder().empty());
+  EXPECT_TRUE(loadOrderHandler.GetLoadOrder().empty());
 }
 
 TEST_P(LoadOrderHandlerTest, getLoadOrderShouldReturnTheCurrentLoadOrder) {
-  initialiseHandler();
-  loadOrderHandler_.LoadCurrentState();
+  auto loadOrderHandler = createHandler();
+  loadOrderHandler.LoadCurrentState();
 
-  ASSERT_EQ(getLoadOrder(), loadOrderHandler_.GetLoadOrder());
-}
-
-TEST_P(LoadOrderHandlerTest,
-       getActivePluginsShouldThrowIfTheHandlerHasNotBeenInitialised) {
-  EXPECT_THROW(loadOrderHandler_.GetActivePlugins(), std::system_error);
+  ASSERT_EQ(getLoadOrder(), loadOrderHandler.GetLoadOrder());
 }
 
 TEST_P(LoadOrderHandlerTest,
        getActivePluginsShouldReturnAnEmptyVectorIfStateHasNotBeenLoaded) {
-  initialiseHandler();
+  auto loadOrderHandler = createHandler();
 
-  EXPECT_TRUE(loadOrderHandler_.GetActivePlugins().empty());
+  EXPECT_TRUE(loadOrderHandler.GetActivePlugins().empty());
 }
+
 TEST_P(LoadOrderHandlerTest, getActivePluginsShouldReturnOnlyActivePlugins) {
-  initialiseHandler();
-  loadOrderHandler_.LoadCurrentState();
+  auto loadOrderHandler = createHandler();
+  loadOrderHandler.LoadCurrentState();
 
-  ASSERT_EQ(getActivePlugins(), loadOrderHandler_.GetActivePlugins());
+  ASSERT_EQ(getActivePlugins(), loadOrderHandler.GetActivePlugins());
 }
 
-TEST_P(LoadOrderHandlerTest,
-       getImplicitlyActivePluginsShouldThrowIfTheHandlerHasNotBeenInitialised) {
-  EXPECT_THROW(loadOrderHandler_.GetImplicitlyActivePlugins(),
-               std::system_error);
-}
 TEST_P(
     LoadOrderHandlerTest,
     getImplicitlyActivePluginsShouldReturnValidDataEvenIfStateHasNotBeenLoaded) {
-  initialiseHandler();
+  auto loadOrderHandler = createHandler();
 
   ASSERT_EQ(getImplicitlyActivePlugins(),
-            loadOrderHandler_.GetImplicitlyActivePlugins());
+            loadOrderHandler.GetImplicitlyActivePlugins());
 
-  loadOrderHandler_.LoadCurrentState();
+  loadOrderHandler.LoadCurrentState();
 
   ASSERT_EQ(getImplicitlyActivePlugins(),
-            loadOrderHandler_.GetImplicitlyActivePlugins());
-}
-
-TEST_P(LoadOrderHandlerTest,
-       setLoadOrderShouldThrowIfTheHandlerHasNotBeenInitialised) {
-  EXPECT_THROW(loadOrderHandler_.SetLoadOrder(loadOrderToSet_),
-               std::system_error);
+            loadOrderHandler.GetImplicitlyActivePlugins());
 }
 
 TEST_P(LoadOrderHandlerTest, setLoadOrderShouldSetTheLoadOrder) {
-  initialiseHandler();
-  loadOrderHandler_.LoadCurrentState();
+  auto loadOrderHandler = createHandler();
+  loadOrderHandler.LoadCurrentState();
 
-  EXPECT_NO_THROW(loadOrderHandler_.SetLoadOrder(loadOrderToSet_));
+  EXPECT_NO_THROW(loadOrderHandler.SetLoadOrder(loadOrderToSet_));
 
   if (GetParam() == GameType::fo4 || GetParam() == GameType::tes5se)
     loadOrderToSet_.erase(begin(loadOrderToSet_));