Modeling a parking lot

Question

Here is my code for the design of a parking lot in C++. The following assumptions are made. Referred from Geeks for Geeks

1. The parking lot has multiple levels. Each level has multiple rows of spots.
2. The parking lot can park motorcycles, cars, and buses.
3. The parking lot has motorcycle spots, compact spots, and large spots.
4. A motorcycle can park in any spot.
5. A car can park in either a single compact spot or a single large spot.
6. A bus can park in five large spots that are consecutive and within the same row. It cannot park in small spots.

---

Vehicle related classes ( excluded trivial implementations ):

enum class VehicleSize { motorcycle = 0, car = 1, bus = 2 };

class Vehicle {
private:
    std::string numberPlate;
public:
    Vehicle() : numberPlate("0000") {};
    Vehicle(std::string numberPlate) : numberPlate(numberPlate) {};
    std::string getNumberPlate() const;
    virtual VehicleSize getSize() const = NULL;
    virtual ~Vehicle();
};

class Bus : public Vehicle {
private:
    const VehicleSize size = VehicleSize::bus;
    static int numOfVehicles;
public:
    Bus() : Vehicle("B" + std::to_string(numOfVehicles)) { ++numOfVehicles; };
    Bus(std::string numberPlate) : Vehicle(numberPlate) { ++numOfVehicles; };;
    virtual VehicleSize getSize() const;
    virtual ~Bus();
};

class Car : public Vehicle {
private:
    const VehicleSize size = VehicleSize::car;
    static int numOfVehicles;
public:
    Car() : Vehicle("C" + std::to_string(numOfVehicles)) { ++numOfVehicles; };
    Car(std::string numberPlate) : Vehicle(numberPlate) { ++numOfVehicles; };
    virtual VehicleSize getSize() const;
    virtual ~Car();
};

class Motorcycle : public Vehicle {
private:
    const VehicleSize size = VehicleSize::motorcycle;
    static int numOfVehicles;
public:
    Motorcycle() : Vehicle("M" + std::to_string(numOfVehicles)) { ++numOfVehicles; };
    Motorcycle(std::string numberPlate) : Vehicle(numberPlate) { ++numOfVehicles; };
    virtual VehicleSize getSize() const;
    virtual ~Motorcycle();
};

ParkingSpot class:

#pragma once

enum SpotSize { Small = 0, Compact = 1, Large = 2 };

class ParkingSpot {
private:
    SpotSize size;
    std::string id;
    int level;
    int row;
    int position;
    Vehicle* vehicle = nullptr;
public:
    ParkingSpot(std::string id, int level, int row, int position, SpotSize size) :
        id(id), level(level), row(row), position(position), size(size) {
    };
    SpotSize getSize() const;
    std::string getSizeName() const;
    Vehicle* getVehicleReference() const;
    std::string getID() const;
    bool isOccupied() const;
    void parkVehicle(Vehicle* vehicle);
    void unparkVehicle();
    virtual ~ParkingSpot() {};

    friend std::ostream &operator<<(std::ostream &out, const ParkingSpot &spot);
};

/********** Implementation ***********/

#include "stdafx.h"
#include "ParkingSpot.h"

using namespace std;

SpotSize ParkingSpot::getSize() const {
    return size;
}

string ParkingSpot::getSizeName() const {
    switch (size) {
    case Small:
        return "Small";
        break;
    case Compact:
        return "Compact";
        break;
    case Large:
        return "Large";
        break;
    default:
        return "Unknown";
    }
}

Vehicle* ParkingSpot::getVehicleReference() const {
    return vehicle;
}

string ParkingSpot::getID() const {
    return id;
}

bool ParkingSpot::isOccupied() const {
    return vehicle != nullptr;
}

void ParkingSpot::parkVehicle(Vehicle * vehicle) {
    this->vehicle = vehicle;
}

void ParkingSpot::unparkVehicle() {
    this->vehicle = nullptr;
}

ostream &operator<<(ostream &out, const ParkingSpot &spot) {
    string isOccupied = spot.isOccupied() ? spot.vehicle->getNumberPlate() : "Unoccupied";
    out << spot.id << " | " << spot.getSizeName() << " | " << isOccupied ; 
    return out;
};

ParkingLot Class:

#pragma once

class ParkingLot {
private:
    std::map<std::string, ParkingSpot*> parkingSpots;
    std::map<std::string, ParkingSpot*> parkedVehicles;
    std::string generateID(int level, int row, int position);
public:
    ParkingLot(std::vector< std::vector< std::pair<int, SpotSize>>> parkingSpots);
    ~ParkingLot();
    void parkVehicle(Vehicle &vehicle);
    std::vector<std::string> getAdjacentSpots(std::string id, int count);
    void unparkVehicle(Vehicle &vehicle);
    bool canFitVehicle(SpotSize spotSize, VehicleSize vehicleSize);
    void printParkingSpots();
};

/******* Implementation ********/
#include "stdafx.h"
#include "ParkingLotManager.h"

using namespace std;

ParkingLot::ParkingLot(vector<vector<pair<int, SpotSize>>> parkingSpots) {
    ParkingSpot* spot;
    string id;
    for (int level = 0; level < parkingSpots.size(); ++level) {
        for (int row = 0; row < parkingSpots[level].size(); ++row) {
            for (int position = 0; position < parkingSpots[level][row].first; ++position) {
                id = generateID(level + 1, row + 1, position + 1);
                spot = new ParkingSpot(id, level + 1, row + 1, position + 1, parkingSpots[level][row].second);
                this->parkingSpots[id] = spot;
            }
        }
    }
}


ParkingLot::~ParkingLot() {
    for (auto it = parkingSpots.begin(); it != parkingSpots.end(); it++) {
        delete (*it).second;
    }
}

void ParkingLot::parkVehicle(Vehicle &vehicle) {
    ParkingSpot *spot = nullptr;
    auto it = parkingSpots.begin();
    vector<string> adjacentSpots;
    while (it != parkingSpots.end()) {
        spot = it->second;
        if (!spot->isOccupied() && canFitVehicle(spot->getSize(), vehicle.getSize())) {

            // If the vehicle is bus, find if the adjacent parking spaces are free and allot accordingly
            if (vehicle.getSize() == VehicleSize::bus) {
                adjacentSpots = getAdjacentSpots(spot->getID(), 4);
                auto spot_it = adjacentSpots.begin();
                for (; spot_it != adjacentSpots.end(); spot_it++) {
                    if (parkingSpots.find(*spot_it) == parkingSpots.end() || parkingSpots[*spot_it]->isOccupied()) {
                        break;
                    }
                }
                if (spot_it != adjacentSpots.end()) {
                    it++;
                    continue;
                } else {
                    for (spot_it = adjacentSpots.begin(); spot_it!=adjacentSpots.end(); spot_it++) {
                        parkingSpots[*spot_it]->parkVehicle(&vehicle);
                    }
                }
            }

            spot->parkVehicle(&vehicle);
            parkedVehicles[vehicle.getNumberPlate()] = spot;
            break;
        }
        it++;
    }
    if (it == parkingSpots.end()) {
        throw "Parking space not available for " + vehicle.getNumberPlate();
    }
}

vector<string> ParkingLot::getAdjacentSpots(string id, int count) {
    vector<string> spots;
    string row_id = id.substr(0, 4);
    int position = stoi(id.substr(4));
    string pos;
    for (int i = 1; i <= count; ++i) {
        pos = (position + i) >= 10 ? to_string(position+i) : '0' + to_string(position + i);
        spots.push_back(row_id + pos);
    }
    return spots;
}

void ParkingLot::unparkVehicle(Vehicle & vehicle) {
    auto it = parkedVehicles.find(vehicle.getNumberPlate());
    if (it == parkedVehicles.end()) {
        throw "The vehicle is not parked anymore.";
        return;
    }
    if (vehicle.getSize() == VehicleSize::bus) {
        vector<string> spots = getAdjacentSpots(it->second->getID(), 4);
        for (int i = 0; i < spots.size(); ++i) {
            parkingSpots[spots[i]]->unparkVehicle();
        }
    }
    it->second->unparkVehicle();
    parkedVehicles.erase(it);
}

bool ParkingLot::canFitVehicle(SpotSize spotSize, VehicleSize vehicleSize) {
    return static_cast<int>(vehicleSize) <= static_cast<int>(spotSize);
}

string ParkingLot::generateID(int level, int row, int position) {
    string result = "";
    result += (level >= 10) ? to_string(level) : '0' + to_string(level);
    result += (row >= 10) ? to_string(row) : '0' + to_string(row);
    result += (position >= 10) ? to_string(position) : '0' + to_string(position);
    return result;
}

void ParkingLot::printParkingSpots() {
    auto it = parkingSpots.begin();
    while (it != parkingSpots.end()) {
        cout << (*(*it).second) << endl;
        it++;
    }
}

Main File:

// ParkingLot.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"
#include <fstream>

using namespace std;

int main()
{
    vector<vector<pair<int, SpotSize>>> parkingSpotsData;
    string parkingDataFileName = "parkingSpots.in";
    ifstream dataFile;
    int levels, rows, numberOfSpots, type;
    pair<int, SpotSize> base_pair;
    dataFile.open(parkingDataFileName);

    if (dataFile) {
        dataFile >> levels;
        for (int i = 0; i < levels; ++i) {
            dataFile >> rows;
            vector<pair<int, SpotSize>> rowsVector;
            for (int j = 0; j < rows; ++j) {
                dataFile >> numberOfSpots >> type;
                base_pair = make_pair(numberOfSpots, SpotSize(type));
                rowsVector.push_back(base_pair);
            }
            parkingSpotsData.push_back(rowsVector);
        }
    }

    dataFile.close();

    ParkingLot myParkingLot(parkingSpotsData);

    int choice = 1;
    string id = "";
    Vehicle *vehicle = nullptr;
    map<string, Vehicle*> vehicles;
    while (choice) {
        cout << endl << "Select an option ; 0 to exit" << endl;
        cout << "1. Park a new motorcycle | 2. Park a new Car | 3. Park a new Bus | 4. Unpark | 5. Print" << endl;
        cin >> choice;
        switch (choice) {
        case 1:
            vehicle = new Motorcycle();
            goto parking;
        case 2:
            vehicle = new Car();
            goto parking;
        case 3:
            vehicle = new Bus();
            goto parking;
        case 4:
            cout << "Enter a vehicle id: ";
            cin >> id;
            id[0] = toupper(id[0]);
            if (vehicles.find(id) != vehicles.end()) {
                try {
                    myParkingLot.unparkVehicle(*(vehicles[id]));
                } catch (const string &e) {
                    cout << e << endl;
                }
            } else {
                cout << "Vehicle ID is invalid" << endl;
            }
            break;
        case -1:
        parking:
            vehicles[vehicle->getNumberPlate()] = vehicle;
            try {
                myParkingLot.parkVehicle(*vehicle);
            } catch (const string&e) {
                cout << e << endl;
            }
            break;
        case 5:
            myParkingLot.printParkingSpots();
            break;
        default:
            cout << "Please enter a valid choice" << endl;
            break;
        }

        if (cin.fail()) {
            cout << "ERROR -- You did not enter an integer";

            // get rid of failure state
            cin.clear();

            // discard 'bad' character(s) 
            cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');

        }
    }

    myParkingLot.printParkingSpots();

    return 0;
}

If anyone is interested in full code, you can find it here.

Answer 1

Some suggestions -

• Take a read through https://en.cppreference.com/w/cpp/language/enum. Unless you have a really good reason (i.e. compatibility with an external API), leave out the =0, =1... in your enum.
• Whenever you have a function block {}, even if it's empty, don't add ; after.
• I'm fairly sure (?) that pure virtual functions cannot accept arbitrary pointer suffixes, so using NULL is misleading (even though it technically produces the desired result). Just use 0.
• In your switch, you have return; break - but the break is not necessary, since the return exits before the break is seen. Maybe you have it in due to a pedantic linter, but I'd drop it.
• The C++ "one true path" is to destroy things instead of explicitly closing them. As such, in your main, you can create a scope block around your use of dataFile. When the object goes out of scope, it will be closed and destroyed.

Answer 2

Focusing on the design:

• Despite the way the question is framed, it is not correct to use an inheritance hierarchy here. Vehicles have no difference in behaviour, and uniformly contain a license plate and size. The parking lot handles the parking logic for all sizes / types of vehicle, which is just fine.

• Although vehicle objects are allocated on the heap, it doesn't look like they are ever deleted.

• Parking spots do not use inheritance, so should not have a virtual destructor (the class could be made final to prevent inheritance). Parking spots can be stored by value, rather than allocated on the heap.

• Storing the parking spot level, row and position inside the id string is error prone and duplicates data. If the level, row and position form a unique id, there's no need for a separate id string.

• Failing to find a parking spot does not seem particularly "exceptional", the function could just return a boolean to indicate success.

---

Reading the parking data from file should be done in a separate function, rather than main. For example:

vector<vector<pair<int, SpotSize>>> parkingSpotsData;

if (!readParkingSpotData("parkingSpots.in", parkingSpotsData)) {
    // throw error / use default data / use empty data
}

ParkingLot myParkingLot(parkingSpotsData);

...

---

Use an enum for user action choices (at least on the code side), rather than a magic number.

The input / main loop should also use functions to properly separate functionality. e.g.:

void runMainLoop(ParkingLot& myParkingLot)
{
    while (true)
    {
        UserAction action = requestUserAction(); // UserAction is an enum.

        switch (action)
        {
        case UserAction::Park:
            doParkAction(myParkingLot); // requests vehicle type, and then parks it.
            break;
        case UserAction::Unpark:
            doUnparkAction(myParkingLot); // requests vehicle id and unparks it.
            break;
        case UserAction::Print:
            doPrintAction(myParkingLot);
            break;
        case UserAction::Exit:
            return;
        }
    }
}

...

    // in main...
    runMainLoop(myParkingLot);

This will make using goto unnecessary.

Note also that the -1 choice in the original code may cause an attempt to dereference a null pointer.

---

To better reflect on the scenario, the vehicle id should probably be entered by the user, rather than generated in the vehicle class. (Which will also make it necessary to check that a vehicle isn't already parked before parking it).

It might be a good idea for motorcycles to prefer smaller spots, so we don't park motorcycles in all of the large spots, and have nowhere for buses to go.

---