Simple Bus Network simulation

Question

enter image description here

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;
 }
 }
 }
}

• Tharindu Kumara: 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.

Answer 1

Here are some observations that may help you improve your code.

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>

Rethink your class design

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(){
 BusSim sim(200);
 sim();
}