#1 - Randomization tests green.

libcron/include/libcron/CronRandomization.h

@@ -61,17 +61,21 @@ namespace libcron
                     std::to_string(left) + "-" + std::to_string(right), numbers);
 
             // Remove items outside limits.
-//            for(auto it = numbers.begin(); it != numbers.end();)
+            if(limit.first != -1 && limit.second != -1)
-//            {
+            {
-//                if(CronData::value_of(*it) < limit.first || CronData::value_of(*it) > limit.second)
+                for (auto it = numbers.begin();
-//                {
+                     it != numbers.end();)
-//                    it = numbers.erase(it);
+                {
-//                }
+                    if (CronData::value_of(*it) < limit.first || CronData::value_of(*it) > limit.second)
-//                else
+                    {
-//                {
+                        it = numbers.erase(it);
-//                    ++it;
+                    }
-//                }
+                    else
-//            }
+                    {
+                        ++it;
+                    }
+                }
+            }
 
             if (res.first)
             {

libcron/src/CronRandomization.cpp

@@ -49,7 +49,7 @@ namespace libcron
             std::set<Months> month_range{};
             if (selected_value == -1)
             {
-                // Month is not specific, we need to get the 'max minimum' day of month to use.
+                // Month is not specific, get the range.
                 CronData cr;
                 res &= cr.convert_from_string_range_to_number_range<Months>(all_sections[5].str(), month_range);
             }
@@ -77,7 +77,7 @@ namespace libcron
 
     std::pair<int, int> CronRandomization::day_limiter(const std::set<Months>& months)
     {
-        auto max = 31;
+        int max = CronData::value_of(DayOfMonth::Last);
 
         for (auto month : months)
         {
@@ -90,12 +90,9 @@ namespace libcron
                                CronData::months_with_31.end(),
                                month) == CronData::months_with_31.end())
             {
+                // Not among the months with 31 days
                 max = std::min(max, 30);
             }
-            else
-            {
-                max = std::min(max, 31);
-            }
         }
 
         auto res = std::pair<int, int>{ CronData::value_of(DayOfMonth::First), max };

test/CronRandomizationTest.cpp

@@ -8,81 +8,71 @@
 
 using namespace libcron;
 
+void test(const char* const random_schedule)
+{
+    libcron::CronRandomization cr;
+    std::unordered_map<int, std::unordered_map<int, int>> results{};
+
+    for (int i = 0; i < 10000; ++i)
+    {
+        auto res = cr.parse(random_schedule);
+        REQUIRE(std::get<0>(res));
+        auto schedule = std::get<1>(res);
+
+        INFO("schedule:" << schedule);
+        Cron<> cron;
+        REQUIRE(cron.add_schedule("validate schedule", schedule, []() {}));
+    }
+}
+
 SCENARIO("Randomize all the things")
 {
-    const char* full_random = "R(0-59) R(0-59) R(0-23) R(1-31) R(1-12) ?";
+    const char* random_schedule = "R(0-59) R(0-59) R(0-23) R(1-31) R(1-12) ?";
-    Cron<> cron;
 
-    GIVEN(full_random)
+    GIVEN(random_schedule)
     {
         THEN("Only valid schedules generated")
         {
-            libcron::CronRandomization cr;
+            test(random_schedule);
-            std::unordered_map<int, std::unordered_map<int, int>> results{};
-
-            for (int i = 0; i < 1000; ++i)
-            {
-                auto res = cr.parse(full_random);
-                REQUIRE(std::get<0>(res));
-                auto schedule = std::get<1>(res);
-
-                auto start = schedule.begin();
-                auto space = std::find(schedule.begin(), schedule.end(), ' ');
-
-                for (int section = 0; start != schedule.end() && section < 5; ++section)
-                {
-                    auto& map = results[section];
-                    auto s = std::string{start, space};
-                    auto value = std::stoi(s);
-                    map[value]++;
-                    start = space + 1;
-                    space = std::find(start, schedule.end(), ' ');
-                }
-
-                REQUIRE(results.size() == 5);
-                INFO("schedule:" << schedule);
-                REQUIRE(cron.add_schedule("validate schedule", schedule, []() {}));
-            }
         }
     }
 }
 
 SCENARIO("Randomize all the things with reverse ranges")
 {
-    // Only generate DayOfMonth up to 28 to prevent failing tests where the month doesn't have more days.
+    const char* random_schedule = "R(45-15) R(30-0) R(18-2) R(28-15) R(8-3) ?";
-    const char* full_random = "R(45-15) R(30-0) R(18-2) R(28-15) 2 ?";
-    Cron<> cron;
 
-    GIVEN(full_random)
+    GIVEN(random_schedule)
     {
         THEN("Only valid schedules generated")
         {
-            libcron::CronRandomization cr;
+            test(random_schedule);
-            std::unordered_map<int, std::unordered_map<int, int>> results{};
+        }
-
+    }
-            for (int i = 0; i < 1000; ++i)
+}
-            {
+
-                auto res = cr.parse(full_random);
+SCENARIO("Randomize all the things - day of week")
-                REQUIRE(std::get<0>(res));
+{
-                auto schedule = std::get<1>(res);
+    const char* random_schedule = "R(0-59) R(0-59) R(0-23) ? R(1-12) R(0-6)";
-
+
-                auto start = schedule.begin();
+    GIVEN(random_schedule)
-                auto space = std::find(schedule.begin(), schedule.end(), ' ');
+    {
-
+        THEN("Only valid schedules generated")
-                for (int section = 0; start != schedule.end() && section < 5; ++section)
+        {
-                {
+            test(random_schedule);
-                    auto& map = results[section];
+        }
-                    auto s = std::string{start, space};
+    }
-                    auto value = std::stoi(s);
+}
-                    map[value]++;
+
-                    start = space + 1;
+SCENARIO("Randomize all the things with reverse ranges - day of week")
-                    space = std::find(start, schedule.end(), ' ');
+{
-                }
+    const char* random_schedule = "R(45-15) R(30-0) R(18-2) ? R(8-3) R(4-1)";
-
+
-                REQUIRE(results.size() == 5);
+    GIVEN(random_schedule)
-                INFO("schedule:" << schedule);
+    {
-                REQUIRE(cron.add_schedule("validate schedule", schedule, []() {}));
+        THEN("Only valid schedules generated")
-            }
+        {
+            test(random_schedule);
         }
     }
 }