views:

180

answers:

4

I have class A and list of A objects. A has a function f that should be executed every X seconds (for the first instance every 1 second, for the seconds instance every 5 seconds, etc.). I have a scheduler class that is responsible to execute the functions at the correct time. What i thought to do is to create a new class, ATime, that will hold ptr to A instance and the time A::f should be executed. The scheduler will hold a min priority queue of Atime.

  1. Do you think it is the correct implementation?
  2. Should ATime be a nested class of the scheduler?
+4  A: 

From what you describe, sounds like it could work :-)

IMHO class ATime belongs to the scheduler more than to A. It is necessary for the scheduler to do its job, and it is not needed for A. In fact, A (and the rest of the world) need not even know about its existence - so putting it a private nested class of the scheduler would be appropriate for me.

Péter Török
A: 

This sort of class is best made a private struct nested within your scheduler class, so you can easily access all fields inside the scheduler class. (All fields of a struct are public by default.)

Frederik Slijkerman
A: 

As to the second part (and the question in the title), IMO it is completely a matter of taste. You could just as well choose to reduce the clutter in the definition of the Scheduler class, and place ATime in a subnamespace with a warning name (like detail) to keep people from using it. After all, if it is only useful for the Scheduler, there's not much need to hide it too much - nobody's going to want to use it anyway.

Perhaps it might be different in C++0x (I think I've heard some rumours how it's going to change the accessibility rules between nested and parent class, or so, in which case nesting might be more worthwhile).

For genericity you might also want to make use of templates, and perhaps end up with Scheduler<A> (using a TimedOperation<A>) (or a myriad of potential refinements/generalizations of that)?

UncleBens
+1  A: 

This may be a little too advanced, but here goes...

boost::function and boost::bind can be used to implement a scheduler that does not need to know anything about class A. This would make your scheduler more generic and reusable.

Here is some sample code that illustrates how those Boost facilities can be used in your case:

#include <ctime>
#include <queue>
#include <boost/function.hpp>
#include <boost/bind.hpp>

struct Foo
{
    void onScheduler(time_t time) {/*...*/}
};

struct Bar
{
    void onScheduler(time_t time) {/*...*/}
};

typedef boost::function<void (time_t)> SchedulerHandler;

struct SchedulerEvent
{
    bool operator<(const SchedulerEvent& rhs) const {return when < rhs.when;}

    SchedulerHandler handler;
    time_t when;
};

class Scheduler
{
public:
    void schedule(SchedulerHandler handler, time_t when)
    {
        SchedulerEvent event = {handler, when};
        queue_.push(event);
    }

private:
    std::priority_queue<SchedulerEvent> queue_;
    void onNextEvent()
    {
        const SchedulerEvent& next = queue_.top();
        next.handler(next.when);
        queue_.pop();
    }
};

int main()
{
    Scheduler s;
    Foo f1, f2;
    Bar b1, b2;

    time_t now = time(0);
    s.schedule(boost::bind(&Foo::onScheduler, &f1, _1), now + 1);
    s.schedule(boost::bind(&Foo::onScheduler, &f2, _1), now + 2);
    s.schedule(boost::bind(&Bar::onScheduler, &b1, _1), now + 3);
    s.schedule(boost::bind(&Bar::onScheduler, &b2, _1), now + 4);

    // Do scheduling...

    return 0;
}

Note that Scheduler knows nothing about Foo & Bar, and vice-versa. All Scheduler really wants is a "callback" functor that matches the signature specified by SchedulerHandler.

If you need a SchedulerEvent to be cancellable, things get a bit complicated because boost::function objects are not comparable. To get around this, you'll need to return some kind of "connection" token when registering events. This is essentially what Boost.Signal does.

Hope this helps.

Emile Cormier