improved version::str() and version::from_string()

include/matador/utils/version.hpp

@@ -3,16 +3,16 @@
 
 #include "matador/utils/result.hpp"
 #include "matador/utils/error.hpp"
+#include "matador/utils/export.hpp"
 
 #include <string>
-#include <ostream>
 
 namespace matador::utils {
 
-class version {
+class MATADOR_UTILS_API version final {
 public:
   version() = default;
-  ~version() =default;
+  ~version() = default;
   version(unsigned int major, unsigned int minor, unsigned int patch);
   version(const version& x) = default;
   version& operator=(const version& x) = default;

source/core/utils/version.cpp

@@ -3,98 +3,122 @@
 #include <matador/utils/errors.hpp>
 
 namespace matador::utils {
+namespace {
 
-version::version(unsigned int major, unsigned int minor, unsigned int patch)
+bool parse_uint_component(const std::string &text, std::size_t &pos, unsigned int &value) {
-: major_(major)
+  if (pos >= text.size() || text[pos] < '0' || text[pos] > '9') {
-, minor_(minor)
+    return false;
-, patch_(patch)
+  }
-{}
 
-bool version::operator==(const version &x) const
+  unsigned int result{};
-{
+
-  return major_ == x.major_ &&
+  while (pos < text.size() && text[pos] >= '0' && text[pos] <= '9') {
-         minor_ == x.minor_ &&
+    const auto digit = static_cast<unsigned int>(text[pos] - '0');
-         patch_ == x.patch_;
+
+    if (result > (std::numeric_limits<unsigned int>::max() - digit) / 10) {
+      return false;
+    }
+
+    result = result * 10 + digit;
+    ++pos;
+  }
+
+  value = result;
+  return true;
 }
 
-bool version::operator!=(const version &x) const
+bool parse_dot(const std::string &text, std::size_t &pos) {
-{
+  if (pos >= text.size() || text[pos] != '.') {
+    return false;
+  }
+
+  ++pos;
+  return true;
+}
+
+} // namespace
+version::version(const unsigned int major, const unsigned int minor, const unsigned int patch)
+: major_(major)
+, minor_(minor)
+, patch_(patch) {
+}
+
+bool version::operator==(const version& x) const {
+  return major_ == x.major_ &&
+    minor_ == x.minor_ &&
+    patch_ == x.patch_;
+}
+
+bool version::operator!=(const version& x) const {
   return !(*this == x);
 }
 
-bool version::operator>(const version &x) const
+bool version::operator>(const version& x) const {
-{
   return !(*this <= x);
 }
 
-bool version::operator>=(const version &x) const
+bool version::operator>=(const version& x) const {
-{
   return !(*this < x);
 }
 
-bool version::operator<(const version &x) const
+bool version::operator<(const version& x) const {
-{
   return (major_ < x.major_) ||
-         (major_ == x.major_ && minor_ < x.minor_) ||
+    (major_ == x.major_ && minor_ < x.minor_) ||
-         (major_ == x.major_ && minor_ == x.minor_ && patch_ < x.patch_);
+    (major_ == x.major_ && minor_ == x.minor_ && patch_ < x.patch_);
 }
 
-bool version::operator<=(const version &x) const
+bool version::operator<=(const version& x) const {
-{
   return *this < x || *this == x;
 }
 
-std::string version::str() const
+std::string version::str() const {
-{
+  return std::to_string(major_) + "." +
-  char buf[32];
+    std::to_string(minor_) + "." +
-  sprintf(buf, "%d.%d.%d", major_, minor_, patch_);
+    std::to_string(patch_);
-  return buf;
 }
 
-std::ostream &operator<<(std::ostream &out, const version &v)
+std::ostream& operator<<(std::ostream& out, const version& v) {
-{
   out << v.str();
   return out;
 }
 
-result<version, error> version::from_string(const std::string &version_string)
+result<version, error> version::from_string(const std::string& version_string) {
-{
+  unsigned int major{};
-  version result;
+  unsigned int minor{};
-  if (const auto ret = sscanf(version_string.c_str(), "%u.%u.%u", &result.major_, &result.minor_, &result.patch_); ret != 3) {
+  unsigned int patch{};
+
+  if (std::size_t pos{}; !parse_uint_component(version_string, pos, major) ||
+      !parse_dot(version_string, pos) ||
+      !parse_uint_component(version_string, pos, minor) ||
+      !parse_dot(version_string, pos) ||
+      !parse_uint_component(version_string, pos, patch) ||
+      pos != version_string.size()) {
     return failure(error(utils_error::InvalidVersionString, version_string));
-  }
+      }
 
-  return ok(result);
+  return ok(version{major, minor, patch});}
-}
 
-unsigned int version::major() const
+unsigned int version::major() const {
-{
   return major_;
 }
 
-unsigned int version::minor() const
+unsigned int version::minor() const {
-{
   return minor_;
 }
 
-unsigned int version::patch() const
+unsigned int version::patch() const {
-{
   return patch_;
 }
 
-void version::major(unsigned int m)
+void version::major(unsigned int m) {
-{
   major_ = m;
 }
 
-void version::minor(unsigned int m)
+void version::minor(unsigned int m) {
-{
   minor_ = m;
 }
 
-void version::patch(unsigned int p)
+void version::patch(unsigned int p) {
-{
   patch_ = p;
 }
-
 }

test/core/utils/VersionTest.cpp

@@ -4,7 +4,26 @@
 
 using namespace matador::utils;
 
-TEST_CASE("Test version interface", "[version][interface]") {
+TEST_CASE("Version: Test form string", "[version][from_string]") {
+  REQUIRE(version::from_string("1.2.3").is_ok());
+  REQUIRE(version::from_string("0.0.0").is_ok());
+
+  REQUIRE(version::from_string("1.2").is_error());
+  REQUIRE(version::from_string("1.2.3.4").is_error());
+  REQUIRE(version::from_string("1.2.3abc").is_error());
+  REQUIRE(version::from_string("abc").is_error());
+  REQUIRE(version::from_string("-1.2.3").is_error());
+}
+
+TEST_CASE("Version: Test comparison", "[version][comparison]") {
+  REQUIRE(version(1, 2, 3) == version(1, 2, 3));
+  REQUIRE(version(1, 2, 3) < version(1, 2, 4));
+  REQUIRE(version(1, 2, 3) < version(1, 3, 0));
+  REQUIRE(version(1, 2, 3) < version(2, 0, 0));
+  REQUIRE(version(2, 0, 0) > version(1, 9, 9));
+}
+
+TEST_CASE("Version: Test interface", "[version][interface]") {
   version v0;
 
   REQUIRE(v0.major() == 0);
@@ -38,8 +57,8 @@ TEST_CASE("Test version interface", "[version][interface]") {
   REQUIRE(v2 == v1);
 }
 
-TEST_CASE("Test version parsing", "[version][parse]") {
+TEST_CASE("Version: Test parsing", "[version][parse]") {
-  const auto version_str{"13.67.34"};
+  constexpr auto version_str{"13.67.34"};
   const auto v1 = version::from_string(version_str);
   REQUIRE(v1.is_ok());