Design of a parking lot. Anyy and all suggestions to improve design are welcome.

[kapil2905]

Hi ppl :),

here is my design for simple parking lot, please put your valuable suggestions on how I could improve this design, pros and cons of this design.

assumptions :
1) Single threaded execution environment.
2) This models single centralised control system for car parking.


parking_lot.h

#ifndef _PARKING_LOT_H_
#define _PARKING_LOT_H_

#include <memory>
#include <vector>
#include <unordered_map>
#include <iostream>

class Slot
{
    public:
        Slot(long int);
        virtual ~Slot();
        void AssignCar(const std::string& carNum);
        long int GetId() const;
        void ReleaseSlot();

    private:
        long int id;
        std::string carNum;
};

class ParkingLot
{
    public:
        static ParkingLot& GetParkingLot();

        long int DriveIn(const std::string& carNum); //finds free slot for parking, marks slot as occupied, returns the occupied slot no.
        void DriveOut(const std::string& carNum);  //frees the slot, deletes car details from the slot.
        void ShowAllParked() const;                //shows all the cars currently present in the parking.
        long int LocateCar(const std::string& carNum); //gives slot number of a car given the car's reg. number.
        std::string GetSlotInfo(long int slotId);     //gives slot info like car parked in the slot, location etc.
        virtual ~ParkingLot();

    private:
        ParkingLot();
        ParkingLot(const ParkingLot&) = delete;
        ParkingLot(ParkingLot&&) = delete;
        ParkingLot& operator=(const ParkingLot&) = delete;
        ParkingLot& operator=(ParkingLot&&) = delete;

        static std::vector<std::unique_ptr<Slot>> freeSlots;            //list of free slots
        static std::vector<std::unique_ptr<Slot>> filledSlots;          //list of occupied slots

        static std::unordered_map<std::string, long int> carMap;        //map for car number to slot number relation
        static std::unordered_map<long int, std::string> slotMap;       //map for slot number to car number relation
        
        static const long int MAX_SLOTS;   //maximum capacity of parking lot
        static bool isInitialised;
        static long int totalCars;
};

#endif 

parking_lot.cpp

#include "parking_lot.h"

Slot::Slot(long int id) : id{id}, carNum{""} {}
Slot::~Slot() {}


long int Slot::GetId() const
{
    return id;
}

void Slot::AssignCar(const std::string& carNum)
{
    this->carNum = carNum;
    return;
}

void Slot::ReleaseSlot()
{
    this->carNum = "";
    return;
}



bool ParkingLot::isInitialised{false};
const long int ParkingLot::MAX_SLOTS{20};
long int ParkingLot::totalCars{0};
std::vector<std::unique_ptr<Slot>> ParkingLot::filledSlots;
std::vector<std::unique_ptr<Slot>> ParkingLot::freeSlots;
std::unordered_map<std::string, long int> ParkingLot::carMap;
std::unordered_map<long int, std::string> ParkingLot::slotMap;

ParkingLot::ParkingLot() {}
ParkingLot::~ParkingLot() {}

ParkingLot& ParkingLot::GetParkingLot()
{
    static ParkingLot parkingLot;

    if (!isInitialised)
    {
        isInitialised = true;

        for (register unsigned int i = 0; i < MAX_SLOTS; ++i)
        {
            freeSlots.push_back(std::make_unique<Slot>(i));
        }
    }

    std::cout << freeSlots.size() << std::endl;

    return parkingLot;
}


long int ParkingLot::DriveIn(const std::string& carNum)
{
    if (freeSlots.size() == 0)  return -1;

    filledSlots.push_back(std::move(freeSlots.back()));
    freeSlots.pop_back();

    filledSlots.at(totalCars)->AssignCar(carNum);

    ++totalCars;

    long int id = filledSlots.at(totalCars - 1)->GetId();

    carMap.insert(std::make_pair(std::string{carNum}, id));
    slotMap.insert(std::make_pair(id, std::string{carNum}));

    return id;
}

void ParkingLot::DriveOut(const std::string& carNum)
{
    if (carMap.find(std::string{carNum}) == carMap.end())   return;

    freeSlots.push_back(std::move(filledSlots.back()));
    filledSlots.pop_back();

    freeSlots.at(freeSlots.size() - 1)->ReleaseSlot();

    long int slotId = freeSlots.at(freeSlots.size() - 1)->GetId();

    carMap.erase(std::string{carNum});
    slotMap.erase(slotId);

    return;
}

void ParkingLot::ShowAllParked() const
{
    for (auto it : carMap)
        std::cout << it.first << "  " << it.second << std::endl;
}


long int ParkingLot::LocateCar(const std::string& carNum)
{
    auto it = carMap.find(std::string{carNum});

    if (it == carMap.end()) return -1;
    return it->second;
}


std::string ParkingLot::GetSlotInfo(long int slotId)
{
    auto it = slotMap.find(slotId);

    if(it == slotMap.end()) return "";
    return it->second;
}

Thanks all for any suggestions :)

[coder777]

Following suggestions:

1. A class that has only static variables isn't a real class. So for now forget about 'static'.

2. Provide ParkingLot a public constructor with the capacity. You can have multiple parking lots with different capacities.

3. At the first sight having a map for quickly finding information seems to be a good idea. But having multiple variables for the same information raises the maintaining effort and is actually error prone. So remove the maps and find the information you need in the vector itself.

4. totalCars seems to be unnecessary. What are you trying to achieve with it? Rather remove it.

[kapil2905]

Thanks alot @coder777 for having a look :)

Regarding your suggestion :

1) A class that has only static variables isn't a real class. So for now forget about 'static'.

==> 1') Tnx :) That means we can use non static member variables for a singleton class ?
Is it that static variables should be avoided as far as possible, if yes, why ?

Thanks :)

PS : Is this not the right forum to ask C++ design related questions as I see very less responses to design related questions.
Kindly let me know if there are any better forums for the same.

[coder777]

That means we can use non static member variables for a singleton class ?

Sure, but why singleton?

Is it that static variables should be avoided as far as possible, if yes, why ?

Yes. Static variables are global variables. Global variables should be to avoided because they are extremely error prone.

Is this not the right forum to ask C++ design related questions as I see very less responses to design related questions.

The problem of the responses is rather that general question need more effort to answer...

[kapil2905]

Thanks @coder777 for replying to all my queries.. :)