Simulation of bus route

Theme of my course work in institute is simulation of bus route. Could you determine any faults and defects in this code? I will be very grateful if you give me some advice about one global variable, counters or etc. It's a console version. I have one month to do GUI with help of Qt or Visual C++. I don't use either them until yesterday. I am trying to use Qt now. I think it must be ordinary graph with points as bus stops and lines as roads.

What do you select in my shoes? I work in VS 2012 usually. I have pretty much time if I need to remake all code below.

Autobus.h

```
#pragma once

#include "Passenger.h"
#include "NecessaryHeaders.h"

using namespace std;

class Autobus{
private:
string brand_;
int capacity_;
int max_speed_;
int bus_numberplate_;
bool is_door_open_;
int x_bus_;
int y_bus_;
int current_speed_;

static int counter_bus_;
static string const mas_brand_buses [];
static int const mas_cap_buses [];
static int const mas_max_speed_buses [];

public:
Autobus(string brand, int const capacity, int max_speed)
:brand_(brand), capacity_(capacity), max_speed_(max_speed), is_door_open_(false), bus_numberplate_(counter_bus_ + 1), x_bus_(0), y_bus_(0), current_speed_(max_speed){
counter_bus_++;
}

Autobus(Autobus const &bus)
:brand_(bus.brand_), capacity_(bus.capacity_), max_speed_(bus.max_speed_), is_door_open_(false), bus_numberplate_(bus.bus_numberplate_), x_bus_(0), y_bus_(0), current_speed_(bus.current_speed_), salon(bus.salon){

}

~Autobus(){
}

vector salon;

size_t get_capacity() const;
string get_brand() const;
int get_max_speed() const;
int get_bus_numberplate() const;
bool get_door() const;
int const get_x_bus() const;
int const get_y_bus() const;
int get_current_speed() const;

int set_x_bus(int temp_x);
int set_y_bus(int temp_y);
void open_door();
void shut_door();
int set_current_spe

Here are some things I see that may help you improve your code. Because C++11 is supported in VS 2012, and because it makes a lot of things easier/better/clearer, I've used C++11 features in my answer.

Consider an event-driven simulation strategy

The current simulation code is "bus-centric" in that it considers things from the point of view of the buses. This can work, but you may find it more natural to use an event-driven simulation strategy instead. Generally, a way to do that is to set up all of the objects (buses, passengers, bus stops, etc.) and then to set them all in motion using events. For example, if you have two buses each leaving two different bus stops, you can generate two next events -- bus1 arrives at next stop, and bus2 arrives at its next stop. Knowing the speed of each bus and the distance between any pair of stops, you can assign a timestamp to each event and store it in a priority queue. The simulation runs by processing the priority queue until the queue is empty. That approach will allow you to have buses simultaneously. If you want to also have smooth animation, you might consider using a time-step based strategy instead in which you consider time as a series of discrete steps of fixed duration (say 30 seconds) and update all object states for each time step.

Decide on which GUI based on your needs

I am a dedicated Linux user, so I would naturally choose Qt over Visual C++ as the basis for a GUI. Your needs may be different. In general, I'd say choose Qt if you can because it allows you many more possibilities (Mac, Linux, Android) than a Windows-only solution. However, if there are particular interface needs that are Windows only, and you don't plan on working with anything else, you might choose the native Windows GUI. In either case, there is a lot to learn, but there are many resources available on the internet for learning either or both.

Don't use system("pause")

There are two reasons not to use system("cls") or system("pause"). The first is that it is not portable to other operating systems which you may or may not care about now. The second is that it's a security hole, which you absolutely must care about. Specifically, if some program is defined and named cls or pause, your program will execute that program instead of what you intend, and that other program could be anything. First, isolate these into a seperate functions cls() and pause() and then modify your code to call those functions instead of system. Then rewrite the contents of those functions to do what you want using C++. In this case, a better alternative would be getchar().

Consider making your code cross-platform

It may surprise you to know that your code can be successfully run under Linux once two small changes are made. First, remove all instances of system("pause") as already mentioned. Second change your NecessaryHeaders.h file to check if the platform is Windows:

#ifdef _WIN32
#include 
#endif

Don't specify const on return values

Generally speaking, it doesn't make sense to specify const on a return value of a function when that return values is a simple type such as int. In particular, the following functions should not have const as part of the return value:

Passenger::get_count_pas()
Autobus::get_x_bus()
Autobus::get_y_bus()

The reason is that the return value is a copy of something internal and not a pointer to anything inside the class, so there's no need to restrict what the calling function can do with the value since it can't affect the class.

Specify constructor items in declaration order

In the Autobus.h file, is this constructor:

Autobus(string brand, int const capacity, int max_speed)
    :brand_(brand), capacity_(capacity), max_speed_(max_speed), is_door_open_(false), bus_numberplate_(counter_bus_ + 1), x_bus_(0), y_bus_(0), current_speed_(max_speed){
        counter_bus_++;
}

However, is_door_open_ is declared after bus_numberplate_ and classes are initialized in declaration order, not in the order in which they're listed in the constructor. To avoid confusion, you should always order constructor items in the same order as they're declared. In this case, the simplest fix is to simply swap is_door_open_ and bus_numberplate_

Eliminate "magic numbers"

When there is code, such as in Autobus::generate_buses that uses numbers like this:

for(int i = 0; i < 3; ++i){
    int temp_rand = 0;
    temp_rand = rand() % 5 + 1;
    // etc.
}

it's not clear what 3 signifies or what 5 signifies. It's generally better to either use named constants or to calculate those numbers, reducing the chance for error when the program is modified.

Consider using a struct for Autobus initialization

In the Autobus::generate_buses code, I see a few areas that might be improved. The code currently looks like this:

```
void Autobus::generate_buses(deque &buses){
for(int i = 0; i < 3; ++i){

StackExchange Code Review Q#71224, answer score: 4