frozen icon indicating copy to clipboard operation
frozen copied to clipboard

Standard implementation for lower_bound/upper_bound/equal_range.

Open nbrachet opened this issue 3 years ago • 4 comments

Namely (quoting cppreference.com):

  • lower_bound "Returns an iterator pointing to the first element that is not less than (i.e. greater or equal to) key."
  • upper_bound "Returns an iterator pointing to the first element that is greater than key."
  • equal_range "Returns a range containing all elements with the given key in the container. The range is defined by two iterators, one pointing to the first element that is not less than key and another pointing to the first element greater than key."

nbrachet avatar May 26 '21 15:05 nbrachet

@nbrachet this is great timing - we just ran into the same issue and I was about to make this change myself when I found your PR. I suggest adding some improved test coverage as follows:

  auto lower_bound = ze_map.lower_bound(0);
  REQUIRE(lower_bound == ze_map.begin());

  lower_bound = ze_map.lower_bound(5);
  REQUIRE(lower_bound == ze_map.begin());

  lower_bound = ze_map.lower_bound(20);
  REQUIRE(lower_bound == ze_map.begin() + 1);

  lower_bound = ze_map.lower_bound(25);
  REQUIRE(lower_bound == ze_map.begin() + 2);

  lower_bound = ze_map.lower_bound(30);
  REQUIRE(lower_bound == ze_map.begin() + 2);

  lower_bound = ze_map.lower_bound(40);
  REQUIRE(lower_bound == ze_map.end());

  auto upper_bound = ze_map.upper_bound(0);
  REQUIRE(upper_bound == ze_map.begin());

  upper_bound = ze_map.upper_bound(10);
  REQUIRE(upper_bound == ze_map.begin() + 1);

  upper_bound = ze_map.upper_bound(15);
  REQUIRE(upper_bound == ze_map.begin() + 1);

  upper_bound = ze_map.upper_bound(30);
  REQUIRE(upper_bound == ze_map.end());

  upper_bound = ze_map.upper_bound(40);
  REQUIRE(upper_bound == ze_map.end());

adamshapiro0 avatar May 27 '21 21:05 adamshapiro0

Oh and actually one more test change to test for the at() fix:

   SECTION("Lookup failure") {
+    REQUIRE(frozen_map.find(3) == frozen_map.end());
+    REQUIRE_THROWS_AS(frozen_map.at(3), std::out_of_range);
     REQUIRE(frozen_map.find(5) == frozen_map.end());
     REQUIRE_THROWS_AS(frozen_map.at(5), std::out_of_range);
   }

adamshapiro0 avatar May 27 '21 21:05 adamshapiro0

Thanks for the suggestions. Added in https://github.com/serge-sans-paille/frozen/pull/125/commits/c445fef5a4c8422a521575c7ee3aa54727322edf

nbrachet avatar May 28 '21 13:05 nbrachet

Thanks for this fix! Can you rebase it against master branch?

serge-sans-paille avatar Jun 13 '21 20:06 serge-sans-paille