Elevator design implementation in C++

Question

I have written c++ code which performs an elevator's function like carrying people from one floor to another. It is a fun project and I have written to test my C++ skills. Please review this code and suggest improvements. Also suggest some other features I should add.

#include <iostream>
#include <algorithm>
#include <vector>
#include <string>

class Elevator
{
    enum Direction{ UP, DOWN };
    Direction direction;

    std::vector<int> requests = {};
    int min_floor; //Undergroud floor will be shown using negative value
    int max_floor;
    int current_floor = 0; //ground floor
    std::size_t current_capacity = 0;
    std::size_t max_capacity;

 public:
    Elevator() = default;
    Elevator(int min_floor, int max_floor, std::size_t max_capacity) :
            min_floor(min_floor),
            max_floor(max_floor),
            max_capacity(max_capacity)
            {}
    ~Elevator() {};

    void start_elevator();

 private:
    void set_initial_request();
    void set_request();
    bool is_valid_request(int floor) const;
    void set_direction();

    int get_min_floor() const
    {
        return min_floor;
    }

    int get_max_floor() const
    {
        return max_floor;
    }

    int get_current_floor() const
    {
        return current_floor;
    }

    void set_current_floor(int floor)
    {
        current_floor = floor;
    }

    std::size_t get_max_capacity() const
    {
        return max_capacity;
    }

    std::size_t get_current_capacity() const
    {
        return current_capacity;
    }

    void set_current_capacity(std::size_t cap)
    {
        current_capacity = cap;
    }
 }; 

    void Elevator::set_initial_request() 
    {
        std::size_t num_of_reqs;
        int dest_floor;

        std::cout << "\nEnter number of requests \n";//<< get_current_floor() << "\n";
        std::cin >> num_of_reqs;

        std::cout << "\nEnter destination floor number.\n";
        std::cin >> dest_floor;
        requests.emplace_back(dest_floor);
        set_current_capacity(1 + get_current_capacity());
        set_direction();

        for (std::size_t i = 1; i < num_of_reqs; ++i)
        {
            std::cin >> dest_floor;
            if (is_valid_request(dest_floor))
            {
                requests.emplace_back(dest_floor);
                set_current_capacity(1 + get_current_capacity());
            }

            if (get_current_capacity() == get_max_capacity())
            {
                std::cout << "No more entry. Elevator is full!!\n";
                break;
            }
        }
    }

    void Elevator::set_request()
    {
        std::size_t num_of_reqs;
        int dest_floor;

        std::cout << "\nEnter number of requests \n";
        std::cin >> num_of_reqs;

        std::cout << "\nEnter destination floor number.\n";
        for (std::size_t i = 0; i < num_of_reqs; ++i)
        {
            std::cin >> dest_floor;
            if (is_valid_request(dest_floor))
            {
                requests.emplace_back(dest_floor);
                set_current_capacity(1 + get_current_capacity());
            }

            if (get_current_capacity() == get_max_capacity())
            {
                std::cout << "No more entry. Elevator is full!!\n";
                break;
            }
        }
    }

    bool Elevator::is_valid_request(int floor) const
    {
        if (get_current_capacity() >= get_max_capacity())
        {
            std::cout << "Elevator is Full!!\n";
            return false;
        }
        else if (direction == UP && floor < get_current_floor())
        {
            std::cout << "Elevator is going UP.\n";
            return false;
        }
        else if (direction == DOWN && floor > get_current_floor())
        {
            std::cout << "Elevator is going DOWN.\n";
            return false;
        }
        else if (floor > get_max_floor() || floor < get_min_floor())
        {
            std::cout << "This floor does not exist\n";
            return false;
        }
        else
        {
            return true;
        }
    }

    void Elevator::set_direction()
    {
        if (requests[0] > get_current_floor())
        {
            direction = UP;
        }
        else if (requests[0] < get_current_floor())
        {
            direction = DOWN;
        }
    }

    void Elevator::start_elevator()
    {
        int curr_floor = get_current_floor();
        std::size_t curr_capacity = get_current_capacity();

        std::cout << "\nThe current floor is " << curr_floor << " and number of person in elevator are " << curr_capacity <<"\n";

        //Entering requests for first time
        set_initial_request(); 
        std::sort(requests.begin(), requests.end());

        while (!requests.empty())
        {
            if (direction == UP)
            {
                set_current_floor(requests[0]);
            }
            else if (direction == DOWN)
            {
                set_current_floor(requests[requests.size() - 1]);
            }
            curr_floor = get_current_floor();
            curr_capacity = get_current_capacity();

            auto curr_floor_req = std::find(requests.begin(), requests.end(), get_current_floor());
            while (curr_floor_req != requests.end())
            {
                requests.erase(curr_floor_req); //removing current floor's requests
                curr_capacity--;
                curr_floor_req = std::find(requests.begin(), requests.end(), get_current_floor());
            }

            set_current_capacity(curr_capacity);

            std::string dir;
            if (direction == UP)
            {
                dir = "UP";
            }
            else
            {       
                dir = "DOWN";
            }

            //Entering requests for current floor
            std::cout << "\n=======================================================\n";
            std::cout << "The current floor is " << curr_floor << " and number of person in elevator are " << curr_capacity <<"\n";
            std::cout << "\nDirection of elevator is " << dir << " and Total capacity of the elevator is " << get_max_capacity() << "\n";
            std::cout << "\nMinimum floor number is " << get_min_floor() << " and Maximum floor number is " << get_max_floor() << "\n";
            std::cout << "\n=======================================================\n";

            if (curr_floor == get_max_floor())
            {
                direction = DOWN;
            }
            else if (curr_floor == get_min_floor())
            {
                direction = UP;
            }

            if (current_capacity == 0) //Elevator is empty
            {
                set_initial_request();
                std::sort(requests.begin(), requests.end());
            }
            else
            {
                set_request();
                std::sort(requests.begin(), requests.end());
            }
        }
    }

int main()
{
    int min_floor_num, max_floor_num;
    std::size_t max_capacity;

    std::cout << "Enter minimum floor number, maximum floor number in the building\n";
    std::cin >> min_floor_num >> max_floor_num;

    std::cout << "Enter maximum capacity for the elevator\n";
    std::cin >> max_capacity;

    Elevator elevator(min_floor_num, max_floor_num, max_capacity);
    elevator.start_elevator();  
}

When people enter in elevator at any floor, the user should know how many people are entering and it is entered in num_of_reqs. How to write code such that we do not need variable num_of_reqs.

Answer 1

Here are some ideas and observations that may help you improve your code. There are some good things here, too. The code uses const appropriately and consistently, which is good.

Separate input, output and calculation

To the degree practical it's usually good practice to separate input, output and calculation for programs like this. By putting them in separate functions, it isolates the particular I/O for your platform (which is likely to be unique to that platform or operating system) from the logic of the simulation (which does not depend on the underlying OS). For example, right now, is_valid_request() returns a boolean value, but also has a side effect of printing to std::cout. I'd suggest that a different method might be to either return a number as with many standard library calls, such that 0 means success and non-zero indicates an error. Which error is determined by the number which then could be printed to the console or displayed in a GUI dialog box.

Format your code consistently

It doesn't matter as much what style you use as it matters that you have a consistent style. Here, the formatting is not too bad, but the different indentation levels are not typical. For example, I would not indent all of those member function definitions. Also there is a spurious semicolon after the destructor:

~Elevator() {};

Let the compiler create default destructor

The compiler will create a destructor by default which is essentially identical to what you've got, so you can simply omit both the declaraton and implementation from your code.

Don't write getters and setters for every class

C++ isn't Java and writing getter and setter functions for every C++ class is not good style. Consider these two private member functions, for example:

int get_current_floor() const
{
        return current_floor;
}

void set_current_floor(int floor)
{
        current_floor = floor;
}

Since there is no error checking or enforcement of invariants here (that is, set_current_floor would happily accept any integer value and doesn't enforce that it must be within the elevator's range), there is no advantage over simply accessing the current_floor data member directly.

Think of the user

There is no graceful way to end the program. Also, there are some strange things such as when we ask to put 1000 people aboard a small elevator. It would be more sensible from the user's perspective, if they were told immediately that a maximum of \$x\$ number of requests will be accepted. That also answers your question about eliminating num_of_reqs: just keep accepting input until either the elevator can't hold any more people or until the user indicates no more input such as by inputting a special sentinel value that indicates end of data.

Understand standard containers

The use of std::vector::emplace_back everywhere in this program is peculiar. Specifically emplace_back constructs a new object and avoids an extra copy or move versus using push_back which can make a difference with a vector of large or complex objects. However here, the only vector is of int, so push_back would be much more appropriate.

Sanitize user input better

If I enter a string such as "Edward" when asked for the minimum floor number, the program stays in an endless loop. It would be better to read a (text) line in and then convert it to a number. Also, there is no check to make sure that max_floor_num is actually greater than min_floor_num. Users can do funny things and you want your program to be robust. Even better might be to have the computer also simulate passengers and only print out results.

Combine stream output operations

The code currently contains this sequence:

std::cout << "\n=======================================================\n";
std::cout << "The current floor is " << curr_floor << " and number of person in elevator are " << curr_capacity <<"\n";
std::cout << "\nDirection of elevator is " << dir << " and Total capacity of the elevator is " << get_max_capacity() << "\n";
std::cout << "\nMinimum floor number is " << get_min_floor() << " and Maximum floor number is " << get_max_floor() << "\n";
std::cout << "\n=======================================================\n";

But you don't really need to do it that way. Instead it could be written like this:

std::cout << "\n=======================================================\n"
    "The current floor is " << curr_floor 
    << " and number of people in the elevator is " << curr_capacity 
    << "\n\nDirection of elevator is " << dir 
    << " and Total capacity of the elevator is " << get_max_capacity() 
    << "\n\nMinimum floor number is " << get_min_floor() 
    << " and Maximum floor number is " << get_max_floor() 
    << "\n\n=======================================================\n";

This makes it more clear it's just one long message sent to std::cout and also uses constant string concatenation because the compiler automatically concatenates the first two string literals together. As a minor change, I've also changed "number of person in elevator are" to the grammatically correct "number of people in the elevator is".

Don't Repeat Yourself (DRY)

The code for set_initial_request() and set_request() is largely the same. The only differences are that the former also calls set_direction() and insists on getting at least one destination floor number even if the user specifies zero requests. Instead, I'd suggest that set_direction() could be called outside this function or called every time and that function modified to only change direction if there are no current passengers.

Use better naming

The name current_capacity is a bit misleading, since "capacity" means, in English, the maximum amount something can hold. It also means max_capacity is redundant. Instead, I'd suggest naming them passengers and capacity.

Reconsider the constructors

The Elevator class has two constructors: one with parameters and one without. I would suggest eliminating the default constructor with no parameters because it would not result in a useful object and there's no way to change the parameters after constructing the Elevator.

Consider additional refinements and features

I've already mentioned that it might be nice to have the computer simulate passenger behavior. Other things to consider are whether UP and DOWN are sufficient. Elevators typically have two indicator lights, one for "up", and one for "down". However, both may be off, indicating that the elevator is free to take in either direction. Also, consider gathering statistics. How far did the elevator travel in total? What was the average waiting time for passengers? What about simulating a bank of elevators? What if some are express elevators serving, for example, only the lobby and top half of the building? There are all kinds of interesting things one could do with this concept.