views:

108

answers:

2

I'm not a professional programmer, so please don't hesitate to state the obvious.

My goal is to use a std::multiset container (typedef EventMultiSet) called currentEvents to organize a list of structs, of type Event, and to have members of class Host occasionally add new Event structs to currentEvents. The structs are supposed to be sorted by one of their members, time. I am not sure how much of what I am trying to do is legal; the g++ compiler reports (in "Host.h") "error: 'EventMultiSet' has not been declared." Here's what I'm doing:

// Event.h
struct Event {
  public:

  bool operator < ( const Event & rhs ) const {
  return ( time < rhs.time );
 }

 double time;
 int eventID;
 int hostID;
};

// Host.h
...
void calcLifeHist( double, EventMultiSet * ); // produces compiler error
...
void addEvent( double, int, int, EventMultiSet * ); // produces compiler error

// Host.cpp
#include "Event.h"
...

// main.cpp
#include "Event.h"
...
typedef std::multiset< Event, std::less< Event > > EventMultiSet;
EventMultiSet currentEvents;
EventMultiSet * cePtr = &currentEvents;
...

Major questions

  1. Where should I include the EventMultiSet typedef?
  2. Are my EventMultiSet pointers obviously problematic?
  3. Is the compare function within my Event struct (in theory) okay?

Thank you very much in advance.

+2  A: 
  1. The compiler errors are simply because your typedef is in the wrong place - only main.cpp knows about it. It looks like you probably want it in Event.h, which both of the others include.

  2. I'm not sure exactly what you're asking - but possibly you want to pass by reference not by pointer?

  3. I don't see anything wrong with it - though you might want to provide the other comparisons (>, <=, ...) too.

Jefromi
Thanks. I moved the typedef line to under the struct definition in Event.h, but I'm still getting the same error about EventMultiSet not being declared in Host.h. (Host.h doesn't #include Event.h--only Host.cpp does. When I include Event.h in Host.h, which I'm not sure I'm supposed to do, I get a message about a redefinition of 'struct Event').
Sarah
Okay, well, the one hard fact is that the typedef should only be in one place, and that everything that needs it should include that file. The same is true, of course, for `struct Event`. Do you have it defined in more than one place (Host.h and Event.h?) Also, do all your header files have include guards? http://en.wikipedia.org/wiki/Include_guard (e.g. wrap Event.h in `#ifdef EVENT_H .... #endif`)
Jefromi
I was getting the EventMultiSet declaration error because I had failed to add #include <set> to Event.h! Sorry about that. Thank you for the suggestion of passing by reference. I had also forgotten an include guard for Event.h.
Sarah
On the comparison operator, be aware that any items that have the same time may get shuffled, since as far as the comparison operator is concerned, they are equivalent. This may or may not be a problem, but it bears pointing out. (It depends on the implementation of the sorting algorithm and the particular contents of the thing you're sorting. In the case of adding things to a sorted data structure, adding something with the same timestamp as another thing, it may go before or after.) So- if order is important between things with the same timestamp, you'll need to encode some more info.
dash-tom-bang
Okay, sorry, the error is still there: I still get "EventMultiSet" has not been declared in Host.h, though I have #include "Event.h" in Host.cpp. I'm stumped. (DTB: Thank you. Right now, sorting at that level isn't necessary.)
Sarah
I added #include "Event.h" to Host.h, and that works.
Sarah
+1  A: 

Given that you solicited statements "of the obvious," one thing I noticed is that you didn't #include <set>, which is required in order for your compiler to know what a multiset is, or #include <functional> which is required to know what less means:

// main.cpp
#include "Event.h"
#include <set>
#include <functional>
...
typedef std::multiset< Event, std::less< Event > > EventMultiSet;
EventMultiSet currentEvents;
EventMultiSet * cePtr = &currentEvents;
John Dibling
Doh. Did not know about #include <functional>. Thank you.
Sarah