0

Right now, I'm working on refactoring a program that calls its parts by polling to a more event-driven structure.

I've created sched and task classes with the sced to become a base class of the current main loop. The tasks will be created for each meter so they can be called off of that instead of polling.

Each of the events main calls are a type of meter that gather info and display it. When the program is coming up, all enabled meters get 'constructed' by a main-sub. In that sub, I want to store off the "this" pointer associated with the meter, as well as the common name for the "action routine.

void MeterMaker::Meter_n_Task (Meter * newmeter,) {
  push(newmeter);      // handle non-timed draw events
  Task t  = new Task(now() + 0.5L);
  t.period={0,1U};
  t.work_meter = newmeter;      
  t.work  = [&newmeter](){newmeter.checkevent();};<<--attempt at lambda
  t.flags = T_Repeat;
  t.enable_task();
  _xos->sched_insert(t);
}

A sample call to it:

Meter_n_Task(new CPUMeter(_xos, "CPU "));

've made the scheduler a base class of the main routine (that handles the loop), and I've tried serveral variations to get the task class to be a base of the meter class, but keep running into roadblocks. It's alot like "whack-a-mole" -- pound in something to fix something one place, and then a new probl pops out elsewhere.

Part of the problem, is that the sched.h file that is trying to hold the Task Q, includes the Task header file. The task file Wants to refer to the most "base", Meter class.

The meter class pulls in the main class of the parent as it passes a copy of the parent to the children so they can access the draw routines in the parent.

Two references in the task file are for the 'this' pointer of the meter and the meter's update sub (to be called via this).

void *this_data= NULL;
void (*this_func)() = NULL;

Note -- I didn't really want to store these in the class, as I wanted to use a lamdba in that meter&task routine above to store a routine+context to be used to call the meter's action routine.

Couldn't figure out the syntax.

But am running into other syntax problems trying to store the pointers...such as

  g++: COMPILE lsched.cc
In file included from meter.h:13:0,
                 from ltask.h:17,
                 from lsched.h:13,
                 from lsched.cc:13:
xosview.h:30:47: error: expected class-name before ‘{’ token
 class XOSView : public XWin, public Scheduler {

Like above where it asks for a class, where the classname "Scheduler" is. !?!? Huh? That IS a class name.

I keep going in circles with things that don't make sense... Ideally I'd get the lamba to work right in the Meter_n_Task routine at the top. I wanted to only store 1 pointer in the 'Task' class that was a pointer to my lambda that would have already captured the "this" value ... but couldn't get that syntax to work at all when I tried to start it into a var in the 'Task' class.

This project, FWIW, is my teething project on the new C++... (of course it's simple!.. ;-))... I've made quite a bit of progress in other areas in the code, but this lambda syntax has me stumped...its at times like thse that I appreciate the ease of this type of operation in perl. Sigh.

Not sure the best way to ask for help here, as this isn't a simple question. But thought I'd try!... ;-)

Too bad I can't attach files to this Q.

  • 1
    It's not clear what you're asking but you may be looking for std::function. Also, one of your problems might be related to circular includes, you'll need to do forward declarations somewhere if that's the case. – o11c Jun 8 '14 at 4:59
  • stackoverflow.com/questions/9186510/… if you want to do that. But are you certain you can't do what you want with plain old virtual functions? Your use of void* is scary. Also use -> on pointers, not .. – Mat Jun 8 '14 at 7:21
  • I added the void to get rid of the reference to 'Meter', as it kept saying it wasn't a type (it was a class name). I also tried a forward declaration, but got slapped for trying to use an "incomplete" type when I tried to create a pointer to it. The circular includes are part of this mess -- but when I've gone for simple forward decl's, I end up with either it not being something I can use as a type OR it being incomplete.-------------------And no, I'm not sure I can't do it with plain virtual functions, but would have to explain in more detail to make that more clear (1 way or the other) – Astara Jun 8 '14 at 7:56
  • @Mat -- I might be able to move toward VF's, with more code restructuring. The 'meters' that gather the data then make downcalls into base classes to do the graphing. I didn't want to tackle gathering of data and display in separate calls just yet, though that's likely the way to go for best responsiveness. I'd also like to get real clear on how to use lambda's in c++ as I have used them alot, in Perl, which has both a benefit and a deficit in having type-mutability up to the point that you use or 'output' the value. – Astara Jun 8 '14 at 18:32
5

Your main problem appears to be that your design makes your classes know too much about each other, which will give you headaches with cyclic dependencies and inextricable ownership issues – especially since you're apparently using raw pointers a lot.

Your Task abstraction shouldn't know anything about Meter, or if it does, it should know about an abstract base class for your meters, and rely exclusively on that. And Meter should be completely oblivious of the Task system. This will enable you to have a task and scheduler abstraction that are either completely generic, or only depend on a well-defined interface.

You state in comments that going with an abstract base class and virtual methods would be too heavy, so a "generic" task abstraction should be the way to go. Tools to build that in C++ would either be templates, or in this case using a std::function to hide the actual type (and "nature") of the task to be run.

Here's a minimal task setup, you'll need to add your task state (enabled/disabled, next start time, etc.) in there, and adapt your scheduler to use this: (see How can I store a lambda expression as a field of a class in C++11?)

#include <functional>

struct Task
{
    Task(std::function<bool()> f): func(f) {}
    bool execute()
    {
        return func();
    }
private:
    std::function<bool()> func;
};

You can use this utility to schedule ordinary functions, or lambdas – std::function takes care of hiding the actual type of the callable it handles for you.

Example usage:

#include <iostream>
#include <vector>

#include "task.h"

bool foo()
{
    std::cout << "Fooing" << std::endl;
    return true;
}

struct Meter
{
    Meter(std::string n): name(n) {}
    bool process_events()
    {
        std::cout << "Processing " << name << std::endl;
        return true;
    }
private:
    std::string name;
};

int main()
{
    Meter m { "CPU" };
    std::vector<Task> tasks;
    tasks.emplace_back(foo);
    tasks.emplace_back([&m](){return m.process_events();});

    for (auto& task: tasks)
        task.execute();
}

Now this looks wrong:

class XOSView : public XWin, public Scheduler { ... };

A window isn't a scheduler, a scheduler isn't a window, and a game isn't either of those either. That sort of construct is will give you more headaches going further than you already have. Use composition. Your game should have a UI, and have a scheduler, and a bunch of other stuff. It shouldn't be a UI and a scheduler at the same time.

_xos->sched_insert(t);

Transform that to use a member scheduler object:

_xos->sched().insert(t);

Then figure out whether your meters need to know about the game controller or the UI. If they need both (what I understand from your comment), pass a reference to both during construction until the time you can make a better interface between those.

Multiple inheritance is very easy to get wrong, even more so than plain inheritance (which is difficult enough already). Avoid it unless you know better.


Random advice:

  • Make sure you understand the difference between a class declaration and definition, and what you can do with an incomplete type (on that has been declared but not defined). See for example Does not name a type error in C++, and for more info When can I use a forward declaration?.
  • Don't try and bypass a type error with a void*. Understand your problem and fix it. Types are central to C++, and void* is as close as to a "non-type" as possible. The compiler can't help you with it, can't warn you about type errors when you use that.
  • Be more careful with your syntax. If t is a pointer t.foo() is wrong. This isn't Java. Classes and struct definitions must end with a ; (struct foo {};). Forgetting that semicolon will lead you to very bizarre compiler errors.
  • Don't use new, this isn't Java. Use std::shared_ptr and/or std::unique_ptr with their std::make_shared/std::make_unique helpers. This will same you a lot of memory management issues, and exception-safety gotchas.
  • More reading on lambdas: What is a lambda expression in C++11?, Lambda Expressions
  • Re: classes know too much about each other -- yes. This program is about 10 years old and not one that I wrote. It has "interacting" features that create problems -- like tying gui-display+refresh into 1 step that requires calling the data-input routine (i.e. can't refresh w/o pulling in data and adding another column(so columns don't represent a fixed amount of time, but anytime a measurement was made)... lots of little gotchas like that.. trying to restructure to fix, (and learning v4-c++ at the same time; last c++ use was near 15-20 yrs ago w/v1 of C++). will have to study your info more – Astara Jun 9 '14 at 18:46
  • BTW-@Mat, reading your thoughts on class design. If the 'Meter', is like TV network that people watch results on, -- then the network would be a scheduler of programs or 'views', on events. I certainly understand where you are coming from -- as I often get confused about differences between an object also 'being' an 'X' vs. an object having an 'X'. Yet, in standard OO examples, unique traits are often part of an object as the traits characterize the object. Wouldn't that make them appropriate for inclusion as part of the object? – Astara Jun 11 '14 at 1:46
0

@Mat, above, brought up alot of good points, and I ended up using a 'function' from "std::functional" to define the "work" data-item in "Task". However, there was a fair amount of distracting details about "what was what" (class hierarchy/derivation) and coding details (use of "new", "void *", and syntax issues) detailsnot to use. So while I did upvote your answer, I'm including the solution that I implemented at the time, which worked, as a detailed record of how I solved the issue of passing the data to a lambda [type] function (using std::functional's function). Admittedly this became a roundabout solution, but seemed to better fit the problem.

As a preface, I should say that I find that "typedefs" (or their 'using' equivalents) and helper functions are near "mandatory" in aiding comprehension of what is going on.

In the "driver code", I simplified its calling the setup of different 'meters' to something that looks like:

#ifdef _XXX_METER_
  if (meterclass_on("xxx_meter")) Meter_n_Task(new XXX_Meter());
#endif

The data that I needed to pass was a function that pointed to the work that needed to be done: a data collection function needed to be called independently of the display functions of top-level driver-class. In each class holding a type of 'meter', a 'checkevent' function is called (event being to "collect current data"). So in the class for all meters of type 'XXX', checkevent is the work that needs to be done periodically by the Task Scheduler.

The class constructor, say for "network traffic", "NetMeter", is fed a class "instance" representing all interfaces if a summary is wanted, or fed interface-specific instances for more specific stats. Either way, they are based on "Meter", which is derived from a "Task" class, that defines a general "work" function, as in :

function<void (void) > work{};

In the constructor for a 'meter', work was assigned the value of the data collection routine, "checkevent" as the last (and only) "statement" in the constructor. The other meter definition values were handled in the meter's definition, ala:

NetMeter::NetMeter (const Instance_Spec &instance ) : 
    FieldMeterGraph(instance),
    DataSrc( IOSrc(proc_netdev),    
      Data_ops( {   new SyntaxFn(net_syntax_filter),
                  new PositionMap(select_fields),
                  new Dev_Matcher((const char *)Xresname),
                  new ByDelta(1000U) }), 256*K) {

  work  =   [this](){checkevent();};
}

BTW, to answer why a Meter was a Task: in this framework, a Meter, among other things, represented a periodic task that needed to be done (time-stamped data collection + processing). In the same way, the top level driver (which was a window-interface), was also the task Scheduler. Hopefully that explains why the top level driver code had:

class XOSView : public XWin, public Scheduler { ... };

Finally, the Meter_n_Task function is where the meter got added to the driver's "todo" list, the window, and the "funky" call to the add the meter to the task scheduler:

void MeterMaker::Meter_n_Task (Meter * newmeter) {
  push(newmeter);      // handle non-timed draw events
  _xos->insert_new(newmeter);
  ( (Task *) newmeter)->taskname( ( (Meter *) newmeter)->name() );
}

Remember, the "work" was already "embedded" in the "meter" data as assigned to "work" in derived class's constructor. So the meter's work was wrapped in the meter class, and the which was placed on the scheduler's run-queue by class "Task"'s constructor. Basically, all the setup work is done in constructors, so I'm sure all is ready before anything references the meters.

The Scheduler is started by 'main' after it does its own initialization task.

Anyway, I started on more refactoring after finishing the above, which got interrupted a few years ago (ouch)... Will have to see how that goes.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.