diff --git a/.github/workflows/cmake.yml b/.github/workflows/cmake.yml index 7a6b0ef..dfdc595 100644 --- a/.github/workflows/cmake.yml +++ b/.github/workflows/cmake.yml @@ -22,7 +22,7 @@ jobs: config: - { name: "Windows Latest MSVC (C++11)", - os: windows-latest, + os: windows-2022, build_type: "Debug", cc: "cl", cxx: "cl", @@ -31,7 +31,7 @@ jobs: } - { name: "Windows Latest MSVC (C++17)", - os: windows-latest, + os: windows-2022, build_type: "Debug", cc: "cl", cxx: "cl", diff --git a/include/set.h b/include/set.h index 58f6ac7..b0a5e35 100644 --- a/include/set.h +++ b/include/set.h @@ -606,11 +606,13 @@ namespace fcpp { // minimum.value() -> 1 [[nodiscard]] fcpp::optional_t min() const { - const auto& it = std::min_element(begin(), end()); - if (it != end()) { - return *it; + if (m_set.empty()) { + return fcpp::optional_t(); } - return fcpp::optional_t(); + // The set is already ordered by TCompare, so the minimum is the first + // element (O(1)). Using std::min_element would re-rank by operator<, + // ignoring the custom comparator and costing O(n). + return *begin(); } // Returns the maximum key in the set, if it's not empty. @@ -627,11 +629,13 @@ namespace fcpp { // maximum.value() -> 8 [[nodiscard]] fcpp::optional_t max() const { - const auto& it = std::max_element(begin(), end()); - if (it != end()) { - return *it; + if (m_set.empty()) { + return fcpp::optional_t(); } - return fcpp::optional_t(); + // The set is already ordered by TCompare, so the maximum is the last + // element (O(1)). Using std::max_element would re-rank by operator<, + // ignoring the custom comparator and costing O(n). + return *std::prev(end()); } // Performs the functional `map` algorithm, in which every element of the resulting set is the @@ -1102,17 +1106,9 @@ namespace fcpp { TKey operator[](size_t index) { assert_smaller_size(index); -#ifdef CPP17_AVAILABLE - auto it = std::advance(begin(), index); - return *it; -#else - auto count = 0; auto it = begin(); - while (count++ < index) { - ++it; - } + std::advance(it, index); return *it; -#endif } // Returns a copy of the key at the given sorted position. diff --git a/tests/set_test.cc b/tests/set_test.cc index 6751d9d..3a83a3f 100644 --- a/tests/set_test.cc +++ b/tests/set_test.cc @@ -332,6 +332,35 @@ TEST(SetTest, MaxEmptySet) EXPECT_FALSE(maximum.has_value()); } +// min()/max() must respect the set's own comparator, not operator<. +// The descending comparator orders ints opposite to operator<, so the +// comparator-smallest element is the largest int, and vice versa. +TEST(SetTest, MinRespectsCustomComparator) +{ + const set numbers( + make_stateful_descending_set({1, 4, 2, 5, 8, 3})); + const auto minimum = numbers.min(); + EXPECT_TRUE(minimum.has_value()); + EXPECT_EQ(8, minimum.value()); +} + +TEST(SetTest, MaxRespectsCustomComparator) +{ + const set numbers( + make_stateful_descending_set({1, 4, 2, 5, 8, 3})); + const auto maximum = numbers.max(); + EXPECT_TRUE(maximum.has_value()); + EXPECT_EQ(1, maximum.value()); +} + +TEST(SetTest, NonConstSubscripting) +{ + set set_under_test(std::set({1, 5, 3, 3})); + EXPECT_EQ(1, set_under_test[0]); + EXPECT_EQ(3, set_under_test[1]); + EXPECT_EQ(5, set_under_test[2]); +} + TEST(SetTest, Map) { const set numbers({4, 1, 3});