Moce functionality in main Cron class with added tests.

libcron/CMakeLists.txt

@@ -2,7 +2,7 @@ cmake_minimum_required(VERSION 3.6)
 project(libcron)
 
 set(CMAKE_CXX_STANDARD 14)
-set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wpedantic -fsanitize=address")
+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Wpedantic")
 
 include_directories(externals/date)
 
@@ -15,4 +15,6 @@ add_library(${PROJECT_NAME}
         CronData.cpp
         CronSchedule.cpp
         CronSchedule.h
-        externals/date/date.h DateTime.h Task.cpp)
+        externals/date/date.h
+        DateTime.h
+        Task.cpp)

libcron/Cron.cpp

@@ -1,29 +1,87 @@
 #include <functional>
 #include "Cron.h"
 
+using namespace std::chrono;
+
 namespace libcron
 {
-
+    bool libcron::Cron::add_schedule( std::string name, const std::string& schedule, std::function<void()> work)
-    bool libcron::Cron::add_schedule(const std::string& schedule, std::function<void()> work)
     {
         auto cron = CronData::create(schedule);
         bool res = cron.is_valid();
         if (res)
         {
-            CronSchedule s(cron);
+
-            Task t(std::move(s), std::move(work));
+            Task t{std::move(name), CronSchedule{cron}, std::move(work)};
             if (t.calculate_next())
             {
-                items.emplace(t);
+                tasks.push(t);
             }
         }
 
         return res;
     }
 
-    bool libcron::Cron::has_expired_task(const std::chrono::system_clock::time_point now) const
+    std::chrono::system_clock::duration Cron::time_until_next(std::chrono::system_clock::time_point now) const
     {
-        return !items.empty() && items.top().is_expired(now);
+        system_clock::duration d{};
+        if (tasks.empty())
+        {
+            d = std::numeric_limits<minutes>::max();
+        }
+        else
+        {
+            d = tasks.top().time_until_expiry(now);
+        }
+
+        return d;
+    }
+
+    size_t Cron::execute_expired_tasks(system_clock::time_point now)
+    {
+        std::vector<Task> executed{};
+
+        while(!tasks.empty()
+              && tasks.top().is_expired(now))
+        {
+            executed.push_back(tasks.top());
+            tasks.pop();
+            auto& t = executed[executed.size()-1];
+            t.execute();
+        }
+
+        auto res = executed.size();
+
+        // Place executed tasks back onto the priority queue.
+        std::for_each(executed.begin(), executed.end(), [this, &now](Task& task)
+        {
+            // Must calculate new schedules using second after 'now', otherwise
+            // we'll run the same task over and over if it takes less than 1s to execute.
+            if(task.calculate_next(now + 1s))
+            {
+                tasks.push(task);
+            }
+        });
+
+        print_queue(tasks);
+
+        return res;
+    }
+
+    void Cron::print_queue(std::priority_queue<Task, std::vector<Task>, std::greater<>> queue)
+    {
+        std::vector<Task> v{};
+
+        while( !queue.empty())
+        {
+            auto t = queue.top();
+            queue.pop();
+            v.push_back(t);
+        }
+
+        std::for_each(v.begin(), v.end(), [&queue](auto& task){
+            queue.push(task);
+        });
     }
 
 }

libcron/Cron.h

@@ -3,6 +3,7 @@
 #include <string>
 #include <chrono>
 #include <queue>
+#include <memory>
 #include "Task.h"
 
 namespace libcron
@@ -11,16 +12,22 @@ namespace libcron
     {
 
         public:
-            bool add_schedule(const std::string& schedule, std::function<void()> work);
+            bool add_schedule(std::string name, const std::string& schedule, std::function<void()> work);
 
             size_t count() const
             {
-                return items.size();
+                return tasks.size();
             }
 
-            bool has_expired_task(std::chrono::system_clock::time_point now = std::chrono::system_clock::now()) const;
+            size_t
+            execute_expired_tasks(std::chrono::system_clock::time_point now = std::chrono::system_clock::now());
+
+            std::chrono::system_clock::duration
+            time_until_next(std::chrono::system_clock::time_point now = std::chrono::system_clock::now()) const;
 
         private:
-            std::priority_queue<Task> items{};
+            // Priority queue placing smallest (i.e. nearest in time) items on top.
+            std::priority_queue<Task, std::vector<Task>, std::greater<>> tasks{};
+            void print_queue(std::priority_queue<Task, std::vector<Task>, std::greater<>> queue);
     };
 }

libcron/CronData.h

@@ -59,6 +59,7 @@ namespace libcron
             static CronData create(const std::string& cron_expression);
 
             CronData();
+            CronData(const CronData&) = default;
 
             bool is_valid() const
             {

libcron/CronSchedule.h

@@ -15,6 +15,8 @@ namespace libcron
             {
             }
 
+            CronSchedule(const CronSchedule&) = default;
+
             std::tuple<bool, std::chrono::system_clock::time_point>
             calculate_from(const std::chrono::system_clock::time_point& from) const;
 

libcron/Task.cpp

@@ -1,17 +1,62 @@
+#include <iostream>
 #include "Task.h"
 
+using namespace std::chrono;
+
 namespace libcron
 {
 
     bool Task::calculate_next(std::chrono::system_clock::time_point from)
     {
         auto result = schedule.calculate_from(from);
-        auto res = std::get<0>(result);
+
-        if(res)
+        // In case the calculation fails, the task will no longer expire.
+        valid = std::get<0>(result);
+        if (valid)
         {
             next_schedule = std::get<1>(result);
         }
 
-        return res;
+        return valid;
+    }
+
+    bool Task::is_expired(std::chrono::system_clock::time_point now) const
+    {
+        return valid && time_until_expiry(now) == 0s;
+    }
+
+    std::chrono::system_clock::duration Task::time_until_expiry(std::chrono::system_clock::time_point now) const
+    {
+        system_clock::duration d{};
+
+        // Explicitly return 0s instead of a possibly negative duration when it has expired.
+        if (now >= next_schedule)
+        {
+            d = 0s;
+        }
+        else
+        {
+            d = next_schedule - now;
+        }
+
+        return d;
+    }
+
+    std::string Task::get_status(std::chrono::system_clock::time_point now) const
+    {
+        std::string s = "'";
+        s+= get_name();
+        s += "' expires in ";
+        s += std::to_string(duration_cast<milliseconds>(time_until_expiry(now)).count());
+        s += "ms => ";
+
+        auto dt = CronSchedule::to_calendar_time(next_schedule);
+        s+= std::to_string(dt.year) + "-";
+        s+= std::to_string(dt.month) + "-";
+        s+= std::to_string(dt.day) + " ";
+        s+= std::to_string(dt.hour) + ":";
+        s+= std::to_string(dt.min) + ":";
+        s+= std::to_string(dt.sec) + " UTC";
+        return s;
     }
 }

libcron/Task.h

@@ -11,30 +11,44 @@ namespace libcron
     {
         public:
 
-            Task(CronSchedule schedule, std::function<void()> task)
+            Task(const std::string name, const CronSchedule schedule, std::function<void()> task)
-                    : schedule(std::move(schedule)), task(std::move(task))
+                    : name(std::move(name)), schedule(std::move(schedule)), task(std::move(task))
             {
             }
 
-            Task(const Task&) = default;
+            void execute() const
+            {
+                task();
+            }
+
+            Task(const Task& other) = default;
 
             Task& operator=(const Task&) = default;
 
             bool calculate_next(std::chrono::system_clock::time_point from = std::chrono::system_clock::now());
 
-            bool operator<(const Task& other) const
+            bool operator>(const Task& other) const
             {
-                return next_schedule < other.next_schedule;
+                return next_schedule > other.next_schedule;
             }
 
-            bool is_expired(std::chrono::system_clock::time_point now = std::chrono::system_clock::now()) const
+            bool is_expired(std::chrono::system_clock::time_point now = std::chrono::system_clock::now()) const;
+
+            std::chrono::system_clock::duration
+            time_until_expiry(std::chrono::system_clock::time_point now = std::chrono::system_clock::now()) const;
+
+            std::string get_name() const
             {
-                return now >= next_schedule;
+                return name;
             }
 
+            std::string get_status(std::chrono::system_clock::time_point now) const;
+
         private:
+            std::string name;
             CronSchedule schedule;
             std::chrono::system_clock::time_point next_schedule;
             std::function<void()> task;
+            bool valid = false;
     };
 }

test/CMakeLists.txt

@@ -2,7 +2,7 @@ cmake_minimum_required(VERSION 3.6)
 project(cron_test)
 
 set(CMAKE_CXX_STANDARD 14)
-set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wpedantic -fsanitize=address -lasan")
+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wpedantic -Wextra")
 
 include_directories(
         externals/Catch2/single_include/

test/CronTest.cpp

@@ -1,6 +1,7 @@
 #include <catch.hpp>
 #include <libcron/Cron.h>
 #include <thread>
+#include <iostream>
 
 using namespace libcron;
 using namespace std::chrono;
@@ -10,7 +11,7 @@ std::string create_schedule_expiring_in(hours h, minutes m, seconds s)
     auto now = system_clock::now() + h + m + s;
     auto dt = CronSchedule::to_calendar_time(now);
 
-    std::string res = "";
+    std::string res{};
     res += std::to_string(dt.sec) + " ";
     res += std::to_string(dt.min) + " ";
     res += std::to_string(dt.hour) + " * * *";
@@ -24,6 +25,7 @@ SCENARIO("Adding a task")
     GIVEN("A Cron instance with no task")
     {
         Cron c;
+        auto expired = false;
 
         THEN("Starts with no task")
         {
@@ -32,23 +34,25 @@ SCENARIO("Adding a task")
 
         WHEN("Adding a task that runs every second")
         {
-            REQUIRE(c.add_schedule("* * * * * *",
+            REQUIRE(c.add_schedule("A task", "* * * * * *",
-                                   []()
+                                   [&expired]()
                                    {
-                                       return;
+                                       expired = true;
                                    })
             );
 
             THEN("Count is 1 and task was not expired two seconds ago")
             {
                 REQUIRE(c.count() == 1);
-                REQUIRE_FALSE(c.has_expired_task(system_clock::now() - 2s));
+                c.execute_expired_tasks(system_clock::now() - 2s);
+                REQUIRE_FALSE(expired);
             }
             AND_THEN("Task is expired when calculating based on current time")
             {
+                c.execute_expired_tasks();
                 THEN("Task is expired")
                 {
-                    REQUIRE(c.has_expired_task());
+                    REQUIRE(expired);
                 }
             }
         }
@@ -59,34 +63,128 @@ SCENARIO("Adding a task that expires in the future")
 {
     GIVEN("A Cron instance with task expiring in 3 seconds")
     {
+        auto expired = false;
+
         Cron c;
-        REQUIRE(c.add_schedule(create_schedule_expiring_in(hours{0}, minutes{0}, seconds{3}),
+        REQUIRE(c.add_schedule("A task", create_schedule_expiring_in(hours{0}, minutes{0}, seconds{3}),
-                               []()
+                               [&expired]()
                                {
-                                   return;
+                                   expired = true;
                                })
         );
 
         THEN("Not yet expired")
         {
-            REQUIRE_FALSE(c.has_expired_task());
+            REQUIRE_FALSE(expired);
         }
         AND_WHEN("When waiting one second")
         {
             std::this_thread::sleep_for(1s);
+            c.execute_expired_tasks();
             THEN("Task has not yet expired")
             {
-                REQUIRE_FALSE(c.has_expired_task());
+                REQUIRE_FALSE(expired);
             }
         }
         AND_WHEN("When waiting three seconds")
         {
             std::this_thread::sleep_for(3s);
+            c.execute_expired_tasks();
             THEN("Task has expired")
             {
-                REQUIRE(c.has_expired_task());
+                REQUIRE(expired);
             }
         }
     }
 }
 
+SCENARIO("Task priority")
+{
+    GIVEN("A Cron instance with two tasks expiring in 3 and 5 seconds, added in 'reverse' order")
+    {
+        auto _3_second_expired = 0;
+        auto _5_second_expired = 0;
+
+
+        Cron c;
+        REQUIRE(c.add_schedule("Five", create_schedule_expiring_in(hours{0}, minutes{0}, seconds{5}),
+                               [&_5_second_expired]()
+                               {
+                                   _5_second_expired++;
+                               })
+        );
+
+        REQUIRE(c.add_schedule("Three", create_schedule_expiring_in(hours{0}, minutes{0}, seconds{3}),
+                               [&_3_second_expired]()
+                               {
+                                   _3_second_expired++;
+                               })
+        );
+
+        THEN("Not yet expired")
+        {
+            REQUIRE_FALSE(_3_second_expired);
+            REQUIRE_FALSE(_5_second_expired);
+        }
+
+        WHEN("Waiting 1 seconds")
+        {
+            std::this_thread::sleep_for(1s);
+            c.execute_expired_tasks();
+
+            THEN("Task has not yet expired")
+            {
+                REQUIRE(_3_second_expired == 0);
+                REQUIRE(_5_second_expired == 0);
+            }
+        }
+        AND_WHEN("Waiting 3 seconds")
+        {
+            std::this_thread::sleep_for(3s);
+            c.execute_expired_tasks();
+
+            THEN("3 second task has expired")
+            {
+                REQUIRE(_3_second_expired == 1);
+                REQUIRE(_5_second_expired == 0);
+            }
+        }
+        AND_WHEN("Waiting 5 seconds")
+        {
+            std::this_thread::sleep_for(5s);
+            c.execute_expired_tasks();
+
+            THEN("3 and 5 second task has expired")
+            {
+                REQUIRE(_3_second_expired == 1);
+                REQUIRE(_5_second_expired == 1);
+            }
+        }
+        AND_WHEN("Waiting based on the time given by the Cron instance")
+        {
+            std::this_thread::sleep_for(c.time_until_next());
+            c.execute_expired_tasks();
+
+            THEN("3 second task has expired")
+            {
+                REQUIRE(_3_second_expired == 1);
+                REQUIRE(_5_second_expired == 0);
+            }
+        }
+        AND_WHEN("Waiting based on the time given by the Cron instance")
+        {
+            std::this_thread::sleep_for(c.time_until_next());
+            REQUIRE(c.execute_expired_tasks() == 1);
+
+            std::this_thread::sleep_for(c.time_until_next());
+            REQUIRE(c.execute_expired_tasks() == 1);
+
+            THEN("3 and 5 second task has each expired once")
+            {
+                REQUIRE(_3_second_expired == 1);
+                REQUIRE(_5_second_expired == 1);
+            }
+        }
+
+    }
+}