Merge pull request #51 from lichray/value-semantics

Value semantics

README.md

@@ -349,7 +349,7 @@ Here's what's happening:
 
 ### Parent Parsers
 
-Sometimes, several parsers share a common set of arguments. Rather than repeating the definitions of these arguments, a single parser with all the shared arguments can be added as a parent to another ArgumentParser instance. The ```.add_parents``` method takes a list of ArgumentParser objects, collects all the positional and optional actions from them, and adds these actions to the ArgumentParser object being constructed:
+Sometimes, several parsers share a common set of arguments. Rather than repeating the definitions of these arguments, a single parser with all the common arguments can be added as a parent to another ArgumentParser instance. The ```.add_parents``` method takes a list of ArgumentParser objects, collects all the positional and optional actions from them, and adds these actions to the ArgumentParser object being constructed:
 
 ```cpp
 argparse::ArgumentParser parent_parser("main");

include/argparse.hpp

@@ -35,11 +35,11 @@ SOFTWARE.
 #include <iterator>
 #include <list>
 #include <map>
-#include <memory>
 #include <numeric>
 #include <sstream>
 #include <stdexcept>
 #include <string>
+#include <string_view>
 #include <tuple>
 #include <type_traits>
 #include <variant>
@@ -380,44 +380,56 @@ public:
         .implicit_value(true);
   }
 
+  ArgumentParser(ArgumentParser &&) noexcept = default;
+  ArgumentParser &operator=(ArgumentParser &&) = default;
+
+  ArgumentParser(const ArgumentParser &other)
+      : mProgramName(other.mProgramName),
+        mPositionalArguments(other.mPositionalArguments),
+        mOptionalArguments(other.mOptionalArguments) {
+    for (auto it = begin(mPositionalArguments); it != end(mPositionalArguments);
+         ++it)
+      index_argument(it);
+    for (auto it = begin(mOptionalArguments); it != end(mOptionalArguments);
+         ++it)
+      index_argument(it);
+  }
+
+  ArgumentParser &operator=(const ArgumentParser &other) {
+    auto tmp = other;
+    std::swap(*this, tmp);
+    return *this;
+  }
+
   // Parameter packing
   // Call add_argument with variadic number of string arguments
   template <typename... Targs> Argument &add_argument(Targs... Fargs) {
-    std::shared_ptr<Argument> tArgument =
-        std::make_shared<Argument>(std::move(Fargs)...);
+    auto tArgument = mOptionalArguments.emplace(cend(mOptionalArguments),
+                                                std::move(Fargs)...);
 
-    if (tArgument->mIsOptional)
-      mOptionalArguments.emplace_back(tArgument);
-    else
-      mPositionalArguments.emplace_back(tArgument);
+    if (!tArgument->mIsOptional)
+      mPositionalArguments.splice(cend(mPositionalArguments),
+                                  mOptionalArguments, tArgument);
 
-    for (const auto &mName : tArgument->mNames) {
-      mArgumentMap.insert_or_assign(mName, tArgument);
-    }
+    index_argument(tArgument);
     return *tArgument;
   }
 
   // Parameter packed add_parents method
   // Accepts a variadic number of ArgumentParser objects
-  template <typename... Targs> void add_parents(Targs... Fargs) {
-    const auto tNewParentParsers = {Fargs...};
-    for (const auto &tParentParser : tNewParentParsers) {
-      const auto &tPositionalArguments = tParentParser.mPositionalArguments;
-      std::copy(std::begin(tPositionalArguments),
-                std::end(tPositionalArguments),
-                std::back_inserter(mPositionalArguments));
-
-      const auto &tOptionalArguments = tParentParser.mOptionalArguments;
-      std::copy(std::begin(tOptionalArguments), std::end(tOptionalArguments),
-                std::back_inserter(mOptionalArguments));
-
-      const auto &tArgumentMap = tParentParser.mArgumentMap;
-      for (const auto &[tKey, tValue] : tArgumentMap) {
-        mArgumentMap.insert_or_assign(tKey, tValue);
+  template <typename... Targs> void add_parents(const Targs &... Fargs) {
+    for (auto &tParentParser : {Fargs...}) {
+      for (auto &tArgument : tParentParser.mPositionalArguments) {
+        auto it =
+            mPositionalArguments.insert(cend(mPositionalArguments), tArgument);
+        index_argument(it);
+      }
+      for (auto &tArgument : tParentParser.mOptionalArguments) {
+        auto it =
+            mOptionalArguments.insert(cend(mOptionalArguments), tArgument);
+        index_argument(it);
       }
     }
-    std::move(std::begin(tNewParentParsers), std::end(tNewParentParsers),
-              std::back_inserter(mParentParsers));
   }
 
   /* Call parse_args_internal - which does all the work
@@ -473,7 +485,7 @@ public:
       size_t tLongestArgumentLength = parser.get_length_of_longest_argument();
 
       for (const auto &argument : parser.mPositionalArguments) {
-        stream << argument->mNames.front() << " ";
+        stream << argument.mNames.front() << " ";
       }
       stream << "\n\n";
 
@@ -482,7 +494,7 @@ public:
 
       for (const auto &mPositionalArgument : parser.mPositionalArguments) {
         stream.width(tLongestArgumentLength);
-        stream << *mPositionalArgument;
+        stream << mPositionalArgument;
       }
 
       if (!parser.mOptionalArguments.empty())
@@ -491,7 +503,7 @@ public:
 
       for (const auto &mOptionalArgument : parser.mOptionalArguments) {
         stream.width(tLongestArgumentLength);
-        stream << *mOptionalArgument;
+        stream << mOptionalArgument;
       }
     }
 
@@ -535,7 +547,7 @@ private:
           throw std::runtime_error(
               "Maximum number of positional arguments exceeded");
         }
-        auto tArgument = *(positionalArgumentIt++);
+        auto tArgument = positionalArgumentIt++;
         it = tArgument->consume(it, end);
       } else if (auto tIterator = mArgumentMap.find(tCurrentArgument);
                  tIterator != mArgumentMap.end()) {
@@ -587,11 +599,17 @@ private:
                              std::end(argumentLengths));
   }
 
+  using list_iterator = std::list<Argument>::iterator;
+
+  void index_argument(list_iterator argIt) {
+    for (auto &mName : std::as_const(argIt->mNames))
+      mArgumentMap.emplace(mName, argIt);
+  }
+
   std::string mProgramName;
-  std::vector<ArgumentParser> mParentParsers;
-  std::vector<std::shared_ptr<Argument>> mPositionalArguments;
-  std::vector<std::shared_ptr<Argument>> mOptionalArguments;
-  std::map<std::string, std::shared_ptr<Argument>> mArgumentMap;
+  std::list<Argument> mPositionalArguments;
+  std::list<Argument> mOptionalArguments;
+  std::map<std::string_view, list_iterator, std::less<>> mArgumentMap;
 };
 
 } // namespace argparse

test/main.cpp

@@ -12,4 +12,5 @@
 #include <test_invalid_arguments.hpp>
 #include <test_negative_numbers.hpp>
 #include <test_required_arguments.hpp>
+#include <test_value_semantics.hpp>
 #include <test_issue_37.hpp>

test/test_parent_parsers.hpp

@@ -12,6 +12,7 @@ TEST_CASE("Add parent parsers", "[parent_parsers]") {
   child_parser.add_parents(parent_parser);
   child_parser.parse_args({ "./main", "--verbose"});
   REQUIRE(child_parser["--verbose"] == true);
+  REQUIRE(parent_parser["--verbose"] == false);
 }
 
 TEST_CASE("Add parent to multiple parent parsers", "[parent_parsers]") {
@@ -26,6 +27,7 @@ TEST_CASE("Add parent to multiple parent parsers", "[parent_parsers]") {
   foo_parser.parse_args({ "./main", "--parent", "2", "XXX" });
   REQUIRE(foo_parser["--parent"] == 2);
   REQUIRE(foo_parser["foo"] == std::string("XXX"));
+  REQUIRE(parent_parser["--parent"] == 0);
 
   argparse::ArgumentParser bar_parser("bar");
   bar_parser.add_argument("--bar");

test/test_value_semantics.hpp

@@ -0,0 +1,95 @@
+#pragma once
+#include <argparse.hpp>
+#include <catch.hpp>
+
+TEST_CASE("ArgumentParser is MoveConstructible and MoveAssignable",
+          "[value_semantics]") {
+  GIVEN("a parser that has two arguments") {
+    argparse::ArgumentParser parser("test");
+    parser.add_argument("foo");
+    parser.add_argument("-f");
+
+    WHEN("move construct a new parser from it") {
+      auto new_parser = std::move(parser);
+
+      THEN("the old parser replaces the new parser") {
+        new_parser.parse_args({"test", "bar", "-f", "nul"});
+
+        REQUIRE(new_parser.get("foo") == "bar");
+        REQUIRE(new_parser.get("-f") == "nul");
+      }
+    }
+
+    WHEN("move assign a parser prvalue to it") {
+      parser = argparse::ArgumentParser("test");
+
+      THEN("the old parser is replaced") {
+        REQUIRE_THROWS_AS(parser.parse_args({"test", "bar"}),
+                          std::runtime_error);
+        REQUIRE_THROWS_AS(parser.parse_args({"test", "-f", "nul"}),
+                          std::runtime_error);
+        REQUIRE_NOTHROW(parser.parse_args({"test"}));
+      }
+    }
+  }
+}
+
+TEST_CASE("ArgumentParser is CopyConstructible and CopyAssignable",
+          "[value_semantics]") {
+  GIVEN("a parser that has two arguments") {
+    argparse::ArgumentParser parser("test");
+    parser.add_argument("foo");
+    parser.add_argument("-f");
+
+    WHEN("copy construct a new parser from it") {
+      auto new_parser = parser;
+
+      THEN("the new parser has the old parser's capability") {
+        new_parser.parse_args({"test", "bar", "-f", "nul"});
+
+        REQUIRE(new_parser.get("foo") == "bar");
+        REQUIRE(new_parser.get("-f") == "nul");
+
+        AND_THEN("but does not share states with the old parser") {
+          REQUIRE_THROWS_AS(parser.get("foo"), std::logic_error);
+          REQUIRE_THROWS_AS(parser.get("-f"), std::logic_error);
+        }
+      }
+
+      AND_THEN("the old parser works as a distinct copy") {
+        new_parser.parse_args({"test", "toe", "-f", "/"});
+
+        REQUIRE(new_parser.get("foo") == "toe");
+        REQUIRE(new_parser.get("-f") == "/");
+      }
+    }
+
+    WHEN("copy assign a parser lvalue to it") {
+      argparse::ArgumentParser optional_parser("test");
+      optional_parser.add_argument("-g");
+      parser = optional_parser;
+
+      THEN("the old parser is replaced") {
+        REQUIRE_THROWS_AS(parser.parse_args({"test", "bar"}),
+                          std::runtime_error);
+        REQUIRE_THROWS_AS(parser.parse_args({"test", "-f", "nul"}),
+                          std::runtime_error);
+        REQUIRE_NOTHROW(parser.parse_args({"test"}));
+        REQUIRE_NOTHROW(parser.parse_args({"test", "-g", "nul"}));
+
+        AND_THEN("but does not share states with the other parser") {
+          REQUIRE(parser.get("-g") == "nul");
+          REQUIRE_THROWS_AS(optional_parser.get("-g"), std::logic_error);
+        }
+      }
+
+      AND_THEN("the other parser works as a distinct copy") {
+        REQUIRE_NOTHROW(optional_parser.parse_args({"test"}));
+        REQUIRE_NOTHROW(optional_parser.parse_args({"test", "-g", "nul"}));
+        REQUIRE_THROWS_AS(
+            optional_parser.parse_args({"test", "bar", "-g", "nul"}),
+            std::runtime_error);
+      }
+    }
+  }
+}