Simple Bus Network simulation

I am simulating a simple bus network (as shown in the picture) using event simulation. There are two buses running (starting at 1 and 4) and the travel time between nodes is 10 units. For now, I used the switch-cases to manage the event. However, if I need to expand the network, it might be very confusing to manage all the events with switch-cases technique.

Is there any solution to improve this code?

#include <iostream>
#include <math.h>
#include <stdlib.h>
#include <queue>
#include <time.h>
#include <functional>
#include <vector>
#include <algorithm>

using namespace std;

double Clock, Travel_time_bw_block;

class Event
{

public:

enum EvtType {arrival_1, arrival_2, arrival_3, arrival_4, arrival_5, arrival_6};

Event(EvtType type = arrival_1, int bus_no = 0, double etime = 0.0):
_type(type),_bus_no(bus_no), _etime(etime) {}

EvtType get_type() const
{
return _type;
}
double get_bus_no() const
{
return _bus_no;
}
double get_time() const
{
return _etime;
}

protected:
EvtType _type;
int _bus_no;
double _etime;
};

struct EventLess
{
bool operator()(const Event& lhs, const Event& rhs) const
{
return (lhs.get_time() > rhs.get_time());
}
};

priority_queue <Event, vector<Event>, EventLess> FutureEventList;

void Initialization()
{
// initialize global variables
Clock = 0.0;

Travel_time_bw_block = 10.0;

Event evt1(Event::arrival_1, 1, 0.0);
Event evt2(Event::arrival_4, 2, 0.0);

FutureEventList.push(evt1);
FutureEventList.push(evt2);
}

int main(){

int Simulation_Time = 200;
Initialization();

while(Clock < Simulation_Time){

Event evt = FutureEventList.top();

FutureEventList.pop();
Clock = evt.get_time();

switch(evt.get_type()){

case Event::arrival_1:
{
cout << "bus " << evt.get_bus_no() << " arriving block 1 at: " << Clock << endl;
Event enter_1 = Event(Event::arrival_2, evt.get_bus_no(), Clock + Travel_time_bw_block);
FutureEventList.push(enter_1);
break;
}
case Event::arrival_2:
{
cout << "bus " << evt.get_bus_no() << " arriving block 2 at: " << Clock << endl;
Event enter_2 = Event(Event::arrival_3, evt.get_bus_no(), Clock + Travel_time_bw_block);
FutureEventList.push(enter_2);
break;
}
case Event::arrival_3:
{
cout << "bus " << evt.get_bus_no() << " arriving block 3 at: " << Clock << endl;
Event enter_3 = Event(Event::arrival_4, evt.get_bus_no(), Clock + Travel_time_bw_block);
FutureEventList.push(enter_3);
break;
}

case Event::arrival_4:
{
cout << "bus " << evt.get_bus_no() << " arriving block 4 at: " << Clock << endl;
Event enter_4 = Event(Event::arrival_5, evt.get_bus_no(), Clock + Travel_time_bw_block);
FutureEventList.push(enter_4);
break;
}

case Event::arrival_5:
{
cout << "bus " << evt.get_bus_no() << " arriving block 5 at: " << Clock << endl;
Event enter_5 = Event(Event::arrival_6, evt.get_bus_no(), Clock + Travel_time_bw_block);
FutureEventList.push(enter_5);
break;
}

case Event::arrival_6:
{
cout << "bus " << evt.get_bus_no() << " arriving block 6 at: " << Clock << endl;
Event enter_6 = Event(Event::arrival_1, evt.get_bus_no(), Clock + Travel_time_bw_block);
FutureEventList.push(enter_6);
break;
}

}
}
}

• If you have a long switch statement, replace it with a std::map. Simply you can reduce the size of your code. And your code will be more cleaner. – Tharindu Kumara Dec 20 '16 at 17:54

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid.

Avoid the use of global variables

I see that Clock is used only within main (and Initialization) but it's declared as s global variable. It's generally better to explicitly pass variables your function will need or declare them within the appropriately smallest possible scope rather than using the vague implicit linkage of a global variable. I'd recommend gathering these into a BusSim class instead.

Use standard templates

The priority queue is currently declared like this:

priority_queue <Event, vector<Event>, EventLess> FutureEventList;


This is very strange for several reasons. First EventLess is defined as a struct:

struct EventLess
{
bool operator()(const Event& lhs, const Event& rhs) const
{
return (lhs.get_time() > rhs.get_time());
}
};


Second, it has "less" in the name but uses ">" in the code. It would be an understatement to describe this as counterintuitive! Instead, I'd recommend creating a more typical operator function for the Event class:

bool operator>(const Event& lhs, const Event& rhs)
{
return lhs.get_time() > rhs.get_time();
}


Then declare the priority_queue like this:

std::priority_queue <Event, std::vector<Event>, std::greater<Event> > FutureEventList;


Don't use std::endl if you don't really need it

The difference betweeen std::endl and '\n' is that '\n' just emits a newline character, while std::endl actually flushes the stream. This can be time-consuming in a program with a lot of I/O and is rarely actually needed. It's best to only use std::endl when you have some good reason to flush the stream and it's not very often needed for simple programs such as this one. Avoiding the habit of using std::endl when '\n' will do will pay dividends in the future as you write more complex programs with more I/O and where performance needs to be maximized.

Use only required #includes

The code has a number of #includes that are not needed. This clutters the code and makes it more difficult to read and understand. Only include files that are actually needed. In this code, I believe these are the only ones required:

#include <iostream>
#include <queue>
#include <functional>
#include <vector>


Your comment about this design being confusing to manage is likely to be true the way it's currently written. Instead, I think I'd design things a little differently. In particular, I think I'd have a Bus object for each bus. Each Bus would contain its own collection of BusStops. Then I would have the whole thing encapsulated in a BusSim class. If that is done, the main could look like this:
int main(){