Version helper compare operator fix (#1824)

* Version helper compare operator fix * Simplyfy compare code
microsoft · Apr 1, 2020 · 90a2286 · 90a2286
1 parent a6b7ba4
commit 90a2286
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 40 deletions.
diff --git a/src/common/UnitTests-CommonLib/UnitTestsVersionHelper.cpp b/src/common/UnitTests-CommonLib/UnitTestsVersionHelper.cpp
@@ -50,45 +50,51 @@ namespace UnitTestsVersionHelper
         }
         TEST_METHOD (whenMajorVersionIsGreaterComparationOperatorShouldReturnProperValue)
         {
-            VersionHelper rhs(MAJOR_VERSION_0, MINOR_VERSION_12, REVISION_VERSION_0);
             VersionHelper lhs(MAJOR_VERSION_0 + 1, MINOR_VERSION_12, REVISION_VERSION_0);
+            VersionHelper rhs(MAJOR_VERSION_0, MINOR_VERSION_12, REVISION_VERSION_0);
 
             Assert::IsTrue(lhs > rhs);
         }
         TEST_METHOD (whenMajorVersionIsLesserComparationOperatorShouldReturnProperValue)
         {
-            VersionHelper rhs(MAJOR_VERSION_0 + 1, MINOR_VERSION_12, REVISION_VERSION_0);
             VersionHelper lhs(MAJOR_VERSION_0, MINOR_VERSION_12, REVISION_VERSION_0);
+            VersionHelper rhs(MAJOR_VERSION_0 + 1, MINOR_VERSION_12, REVISION_VERSION_0);
 
             Assert::IsFalse(lhs > rhs);
         }
         TEST_METHOD (whenMajorVersionIsEqualComparationOperatorShouldCompareMinorVersionValue)
         {
-            VersionHelper rhs(MAJOR_VERSION_0, MINOR_VERSION_12 - 1, REVISION_VERSION_0);
-
             VersionHelper lhs(MAJOR_VERSION_0, MINOR_VERSION_12, REVISION_VERSION_0);
+            VersionHelper rhs(MAJOR_VERSION_0, MINOR_VERSION_12 - 1, REVISION_VERSION_0);
 
             Assert::IsTrue(lhs > rhs);
         }
         TEST_METHOD (whenMajorVersionIsEqualComparationOperatorShouldCompareMinorVersionValue2)
         {
-            VersionHelper rhs(MAJOR_VERSION_0, MINOR_VERSION_12, REVISION_VERSION_0);
             VersionHelper lhs(MAJOR_VERSION_0, MINOR_VERSION_12 - 1, REVISION_VERSION_0);
+            VersionHelper rhs(MAJOR_VERSION_0, MINOR_VERSION_12, REVISION_VERSION_0);
 
             Assert::IsFalse(lhs > rhs);
         }
 
         TEST_METHOD (whenMajorAndMinorVersionIsEqualComparationOperatorShouldCompareRevisionValue)
         {
-            VersionHelper rhs(MAJOR_VERSION_0, MINOR_VERSION_12, REVISION_VERSION_0);
             VersionHelper lhs(MAJOR_VERSION_0, MINOR_VERSION_12, REVISION_VERSION_0 + 1);
+            VersionHelper rhs(MAJOR_VERSION_0, MINOR_VERSION_12, REVISION_VERSION_0);
 
             Assert::IsTrue(lhs > rhs);
         }
         TEST_METHOD (whenMajorAndMinorVersionIsEqualComparationOperatorShouldCompareRevisionValue2)
         {
+            VersionHelper lhs(MAJOR_VERSION_0, MINOR_VERSION_12, REVISION_VERSION_0);
             VersionHelper rhs(MAJOR_VERSION_0, MINOR_VERSION_12, REVISION_VERSION_0 + 1);
+
+            Assert::IsFalse(lhs > rhs);
+        }
+        TEST_METHOD (whenMajorMinorAndRevisionIsEqualGreaterThanOperatorShouldReturnFalse)
+        {
             VersionHelper lhs(MAJOR_VERSION_0, MINOR_VERSION_12, REVISION_VERSION_0);
+            VersionHelper rhs(MAJOR_VERSION_0, MINOR_VERSION_12, REVISION_VERSION_0);
 
             Assert::IsFalse(lhs > rhs);
         }

diff --git a/src/common/VersionHelper.cpp b/src/common/VersionHelper.cpp
@@ -30,34 +30,5 @@ VersionHelper::VersionHelper(int major, int minor, int revision) :
 
 bool VersionHelper::operator>(const VersionHelper& rhs)
 {
-    if (major < rhs.major)
-    {
-        return false;
-    }
-    else if (major > rhs.major)
-    {
-        return true;
-    }
-    else
-    {
-        if (minor < rhs.minor)
-        {
-            return false;
-        }
-        else if (minor > rhs.minor)
-        {
-            return true;
-        }
-        else
-        {
-            if (revision < rhs.revision)
-            {
-                return false;
-            }
-            else
-            {
-                return true;
-            }
-        }
-    }
+    return std::tie(major, minor, revision) > std::tie(rhs.major, rhs.minor, rhs.revision);
 }
diff --git a/src/common/msi_to_msix_upgrade_lib/msi_to_msix_upgrade.cpp b/src/common/msi_to_msix_upgrade_lib/msi_to_msix_upgrade.cpp
@@ -110,16 +110,15 @@ std::future<std::optional<new_version_download_info>> check_for_new_github_relea
         winrt::Windows::Foundation::Uri release_page_uri{ json_body.GetNamedString(L"html_url") };
 
         VersionHelper github_version(winrt::to_string(new_version));
-
         VersionHelper current_version(VERSION_MAJOR, VERSION_MINOR, VERSION_REVISION);
 
-        if (current_version > github_version)
+        if (github_version > current_version)
         {
-            co_return std::nullopt;
+            co_return new_version_download_info{ std::move(release_page_uri), new_version.c_str() };
         }
         else
         {
-            co_return new_version_download_info{ std::move(release_page_uri), new_version.c_str() };
+            co_return std::nullopt;
         }
     }
     catch (...)