12
\$\begingroup\$

I wrote a simple room booking system to get to know more about C++ and OOP. It simply stores rooms/labs/lecture halls with name, position, doors and possibly more details and can be booked/unbooked. features include dynamically adding/removing rooms, getting specific query rooms and summarizing the details. The C++ code is at this place. It basically asks for user input. I tried to make it very robust with invalid input fail-proof.

The main things I am unaware are of standard practices, conventions and better ways to get few things done. I have coded most of it using various examples spread around the web, which might not be optimal for this application.

What improvements could have been done?


RoomBookingSystem.h

#ifndef ROOMBOOKINGSYSTEM_H_
#define ROOMBOOKINGSYSTEM_H_

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

#include "Position.h"
#include "Room.h"
#include "Lab.h"
#include "LectureHall.h"

class Lab;
class LectureHall;

using namespace std;

#define ROOMS 1
#define LABS 2
#define LECTURE_HALLS 3

//TODO make readme
class RoomBookingSystem {

        void parse(vector<vector<string> > roomDetails, int type);
        void loadstate(vector<vector<string> > bookstatus, int type);
        vector<Room*>* getList(int type);
        vector<Room*>* getConstList(int type) const;
        template<typename T> Room* getRoom(int type, const T matcher) const;
        template<typename T> void print(vector<Room*>& vec, const string& pDescriptor);
        struct RoomComparator {
                bool operator()(const int i, const int j);
        };
        void saveRooms(vector<Room*>& vec, const char* loc, const char* bookloc);
        vector<vector<string> > read(const char* pFilename, const int pColumns);

    public:

        RoomBookingSystem();
        ~RoomBookingSystem();
        void addRoom(const string& pName, const double pArea, const int pDoors, const int pX, const int pY);
        void addLab(const string& pName, const double pArea, const int pDoors, const int pX, const int pY, int pComputers);
        void addLectureHall(const string& pName, const double pArea, const int pDoors, const int pX, const int pY, const int pChairs);
        bool removeRoom(int type, Room* r);
        Room* getRoom(const string& pName, int type) const;
        Room* getRoom(const int pX, const int pY, int type) const;
        vector<Room*> getMinimumRoomsOfArea(const int pRequiredArea) const;
        LectureHall* getSmallestLectureHallByStudents(const int pRequiredChairs) const;
        Lab* getLabByLeastEquipmentDensity() const;
        Room* getNearestRoom(const int pX, const int pY) const;
        void printAll();

};

#endif /* ROOMBOOKINGSYSTEM_H_ */

RoomBookingSystem.cpp

#include "RoomBookingSystem.h"

#include <stdlib.h>
#include <algorithm>
#include <cfloat>
#include <climits>
#include <fstream>
#include <iterator>
#include <string.h>

#include "Matchers.h"

#define expand_it (*it)[0], atof((*it)[1].c_str()), atoi((*it)[2].c_str()), Position(atoi((*it)[3].c_str()), atoi((*it)[4].c_str()))
#define print_separator cout<<string(100,'=')<<endl;
#define sq(x) (x)*(x)
using namespace std;

#define ROOM_FILE_LOC "data/ROOM.txt"
#define LAB_FILE_LOC "data/LAB.txt"
#define LECTURE_HALL_FILE_LOC "data/LEC.txt"

#define ROOM_BOOK_LOC "data/ROOM_BOOK.txt"
#define LAB_BOOK_LOC "data/LAB_BOOK.txt"
#define LECTURE_HALL_BOOK_LOC "data/LEC_BOOK.txt"

vector<Room*> rooms;
vector<Room*> labss;
vector<Room*> lectureHalls;

RoomBookingSystem::RoomBookingSystem() {
        vector<vector<string> > roomDetails = read(ROOM_FILE_LOC, 5);
        vector<vector<string> > labDetails = read(LAB_FILE_LOC, 6);
        vector<vector<string> > lectureHallDetails = read(LECTURE_HALL_FILE_LOC, 6);
        vector<vector<string> > roomBookDetails = read(ROOM_BOOK_LOC, 1);
        vector<vector<string> > labBookDetails = read(LAB_BOOK_LOC, 1);
        vector<vector<string> > lectureHallBookDetails = read(LECTURE_HALL_BOOK_LOC, 1);
        parse(roomDetails, ROOMS);
        parse(labDetails, LABS);
        parse(lectureHallDetails, LECTURE_HALLS);
        loadstate(roomBookDetails, ROOMS);
        loadstate(labBookDetails, LABS);
        loadstate(lectureHallBookDetails, LECTURE_HALLS);
}

RoomBookingSystem::~RoomBookingSystem() {
        saveRooms(rooms, ROOM_FILE_LOC, ROOM_BOOK_LOC);
        saveRooms(labss, LAB_FILE_LOC, LAB_BOOK_LOC);
        saveRooms(lectureHalls, LECTURE_HALL_FILE_LOC, LECTURE_HALL_BOOK_LOC);
}
vector<vector<string> > RoomBookingSystem::read(const char* pFilename, const int pColumns) {
        ifstream inputStream;
        vector<vector<string> > rooms_list;
        inputStream.open(pFilename);
        if (inputStream.is_open()) {
                while (!inputStream.eof()) {
                        vector<string> roomDetails;
                        for (int i = 0; i < pColumns; i++) {
                                string detail;
                                inputStream >> detail;
                                roomDetails.push_back(detail);
                        }
                        rooms_list.push_back(roomDetails);
                }
        }
        inputStream.close();
        return rooms_list;
}
void RoomBookingSystem::saveRooms(vector<Room*>& vec, const char* loc, const char* bookloc) {
        ofstream outputStream;
        ofstream outputStream2;
        outputStream.open(loc);
        outputStream2.open(bookloc);
        string s = string(loc);
        if (outputStream.is_open() && outputStream2.is_open()) {
                for (unsigned i = 0; i < vec.size(); i++) {
                        Room* r = vec[i];
                        outputStream << r->getName() << "\t" << fixed << setprecision(4) << r->getArea() << "\t" << r->getDoors() << "\t" << r->getPosition().x << "\t" << r->getPosition().y;
                        if (s.compare(LAB_FILE_LOC) == 0) {
                                outputStream << "\t" << ((Lab*) r)->getComputers();
                        } else if (s.compare(LECTURE_HALL_FILE_LOC) == 0) {
                                outputStream << "\t" << ((LectureHall*) r)->getChairs();
                        }
                        outputStream2 << ((vec[i]->isBooked()) ? "1" : "0");
                        if (i != vec.size() - 1) {
                                outputStream << endl;
                                outputStream2 << endl;
                        }
                }
        }
        outputStream.close();
        outputStream2.close();
}
vector<Room*>* RoomBookingSystem::getList(int type) {
        switch (type) {
        case ROOMS:
                return &rooms;
        case LABS:
                return &labss;
        case LECTURE_HALLS:
                return &lectureHalls;
        }
        return NULL;
}
vector<Room*>* RoomBookingSystem::getConstList(int type) const {
        switch (type) {
        case ROOMS:
                return &rooms;
        case LABS:
                return &labss;
        case LECTURE_HALLS:
                return &lectureHalls;
        }
        return NULL;
}

void RoomBookingSystem::parse(vector<vector<string> > roomDetails, int type) {
        for (vector<vector<string> >::iterator it = roomDetails.begin(); it != roomDetails.end(); it++) {
                switch (type) {
                case ROOMS:
                        rooms.push_back(new Room(expand_it));
                        break;
                case LABS:
                        labss.push_back(new Lab(expand_it, atoi((*it)[5].c_str())));
                        break;
                case LECTURE_HALLS:
                        lectureHalls.push_back(new LectureHall(expand_it, atoi((*it)[5].c_str())));
                        break;
                }
        }
}
void RoomBookingSystem::loadstate(vector<vector<string> > bookstatus, int type) {
        vector<Room*>* vec = getList(type);
        for (unsigned i = 0; i < vec->size(); i++) {
                ((*vec)[i])->setBooked(atoi(bookstatus[i][0].c_str()) == 1);
        }
}

void bookRoom(Room &r) {
        r.book();
}
void unbookRoom(Room &r) {
        r.unbook();
}
void RoomBookingSystem::printAll() {
        print<Room>(rooms, "Rooms");
        print<Lab>(labss, "Labs");
        print<LectureHall>(lectureHalls, "Lecture Halls");
}
template<typename T> void RoomBookingSystem::print(vector<Room*>& vec, const string& pDescriptor) {
        print_separator
                cout << "These are currently " << vec.size() << " " << pDescriptor << ": " << endl;
        for (unsigned i = 0; i < vec.size(); i++) {
                ((T*) vec[i])->printRoom();
        }
        print_separator
}
void RoomBookingSystem::addRoom(const string& pName, const double pArea, const int pDoors, const int pX, const int pY) {
        rooms.push_back(new Room(pName, pArea, pDoors, Position(pX, pY)));
}
void RoomBookingSystem::addLab(const string& pName, const double pArea, const int pDoors, const int pX, int pY, int pComputers) {
        labss.push_back(new Lab(pName, pArea, pDoors, Position(pX, pY), pComputers));
}
void RoomBookingSystem::addLectureHall(const string& pName, const double pArea, const int pDoors, const int pX, const int pY, const int pChairs) {
        lectureHalls.push_back(new LectureHall(pName, pArea, pDoors, Position(pX, pY), pChairs));
}
Room* RoomBookingSystem::getRoom(const string& pName, int type) const {
        return getRoom(type, Matchers::MatchName(pName));
}
Room* RoomBookingSystem::getRoom(const int pX, const int pY, int type) const {
        return getRoom(type, Matchers::MatchPosition(pX, pY));
}
bool RoomBookingSystem::RoomComparator::operator()(const int i, const int j) {
        bool b1 = rooms[i]->isBooked();
        bool b2 = rooms[j]->isBooked();
        if (b1 && !b2) {
                return i;
        } else if (!b1 && b2) {
                return j;
        } else {
                return rooms[i]->getArea() >= rooms[j]->getArea();
        }
}
vector<Room*> RoomBookingSystem::getMinimumRoomsOfArea(const int pRequiredArea) const {
        vector<int> roomIndices;
        vector<Room*> requiredRoom;
        for (unsigned i = 0; i < rooms.size(); i++)
                roomIndices.push_back(i);
        sort(roomIndices.begin(), roomIndices.end(), RoomComparator());
        double area = 0;
        for (unsigned i = 0; i < roomIndices.size() && area < pRequiredArea && !rooms[roomIndices[i]]->isBooked(); i++) {
                area += rooms[roomIndices[i]]->getArea();
                requiredRoom.push_back(rooms[roomIndices[i]]);
        }
        if (area >= pRequiredArea)
                return requiredRoom;
        else {
                vector<Room*> emptyVec;
                return emptyVec;
        }
}
LectureHall* RoomBookingSystem::getSmallestLectureHallByStudents(const int pRequiredChairs) const {
        double minArea = DBL_MAX;
        LectureHall* requiredHall = new LectureHall("", 0, 0, Position(0, 0), 0);
        for (vector<Room*>::iterator it = lectureHalls.begin(); it != lectureHalls.end(); it++) {
                if (((LectureHall*) *it)->getChairs() >= pRequiredChairs && !(*it)->isBooked() && (*it)->getArea() < minArea) {
                        requiredHall = (LectureHall*) *it;
                        minArea = requiredHall->getArea();
                }
        }
        return requiredHall;
}
Lab* RoomBookingSystem::getLabByLeastEquipmentDensity() const {
        double minDensity = DBL_MAX;
        Lab* requiredLab = new Lab("", 0, 0, Position(0, 0), 0);
        for (vector<Room*>::iterator it = labss.begin(); it != labss.end(); it++) {
                if (((Lab*) *it)->getEquipmentDensity() < minDensity && !(*it)->isBooked()) {
                        requiredLab = (Lab*) *it;
                        minDensity = requiredLab->getEquipmentDensity();
                }
        }
        return requiredLab;
}

Room* RoomBookingSystem::getNearestRoom(const int pX, const int pY) const {
        long minDistanceSquared = LONG_MAX;
        Room* requiredRoom = new Room("", 0, 0, Position(0, 0));
        for (vector<Room*>::iterator it = rooms.begin(); it != rooms.end(); it++) {
                long distanceSquared = sq((*it)->getPosition().x - pX) + sq((*it)->getPosition().y - pY);
                if (distanceSquared < minDistanceSquared && !(*it)->isBooked()) {
                        requiredRoom = *it;
                        minDistanceSquared = distanceSquared;
                }
        }
        return requiredRoom;
}

template<class T> Room* RoomBookingSystem::getRoom(int type, const T matcher) const {
        vector<Room*>* vec = getConstList(type);
        vector<Room*>::iterator r = find_if(vec->begin(), vec->end(), matcher);
        return *r;
}

bool RoomBookingSystem::removeRoom(int type, Room* r) {
        vector<Room*>* vec = getList(type);
        if (r != *vec->end()) {
                vec->erase(std::remove(vec->begin(), vec->end(), r), vec->end());
                return true;
        } else
                return false;
}

Room.h

#ifndef ROOM_H_
#define ROOM_H_

#include <string>
#include <iomanip>

#include "Position.h"

#define expand_self "Name: "<<name<<"\tArea: "<<setprecision(4)<<area<<"\tDoors: "<<doors<<"\tPosition: ("<<position.x<<","<<position.y<<")\tBooked: "<<((booked)?"Yes":"No")

using namespace std;

class Room {

    protected:

        string name;
        double area;
        int doors;
        Position position;
        bool booked;

    public:

        Room(string pName, double pArea, int pDoors, Position pPosition);
        virtual void printRoom();
        virtual ~Room() {
        }
        bool operator==(const Room& b);
        double getArea() const;
        bool isBooked() const;
        void book();
        void unbook();
        int getDoors() const;
        const string& getName() const;
        Position getPosition() const;

        void setBooked(bool booked);
};

#endif /* ROOM_H_ */

Room.cpp

#include "Room.h"

#include <iostream>

using namespace std;
Room::Room(string pName, double pArea, int pDoors, Position pPosition) :
        name(pName), area(pArea), doors(pDoors), position(pPosition), booked(false) {
}
void Room::printRoom() {
    cout << expand_self << endl;
}
bool Room::operator==(const Room& r) {
    return name == r.name && position.x == r.position.x && position.y == r.position.y;
}
double Room::getArea() const {
    return area;
}

bool Room::isBooked() const {
    return booked;
}

void Room::book() {
    this->booked = true;
}
void Room::unbook() {
    this->booked = false;
}

int Room::getDoors() const {
    return doors;
}

const string& Room::getName() const {
    return name;
}

Position Room::getPosition() const {
    return position;
}
void Room::setBooked(bool booked) {
    this->booked = booked;
}

Position.h

#ifndef POSITION_H_
#define POSITION_H_

using namespace std;

class Position {

    public:
        int x;
        int y;
        Position(const int a, const int b);
};

#endif /* POSITION_H_ */

Position.cpp

#include "Position.h"

using namespace std;
Position::Position(int a, int b) :
        x(a), y(b) {
}

Matchers.h

#ifndef MATCHERS_H_
#define MATCHERS_H_

#include <string>

#include "Position.h"
#include "Room.h"

using namespace std;

class Matchers {
    public:
        struct MatchName {
                MatchName(const string& s);
                bool operator()(const Room* r) const;
            private:
                const string& name;
        };
        struct MatchPosition {
                MatchPosition(const int pX, const int pY);
                bool operator()(const Room* r) const;
            private:
                const int x;
                const int y;
        };
};

#endif /* MATCHERS_H_ */

Matchers.cpp

#include "Matchers.h"

using namespace std;
Matchers::MatchName::MatchName(const string& s) :
        name(s) {
}
bool Matchers::MatchName::operator()(const Room* r) const {
    return r->getName() == name;
}
Matchers::MatchPosition::MatchPosition(const int pX, const int pY) :
        x(pX), y(pY) {
}
bool Matchers::MatchPosition::operator()(const Room* r) const {
    return r->getPosition().x == x && r->getPosition().y == y;
}

LectureHall.h

#ifndef LECTUREHALL_H_
#define LECTUREHALL_H_

#include <string>

#include "Room.h"

using namespace std;

class LectureHall: public Room {

        int chairs;
    public:
        LectureHall(string pName, double pArea, int pDoors, Position pPosition, int pChairs);
        virtual void printRoom();
        virtual ~LectureHall() {
        }
        int getChairs() const;
        void setChairs(int chairs);
};

#endif /* LECTUREHALL_H_ */

LectureHall.cpp

#include "LectureHall.h"

#include <iostream>

#include "Position.h"

using namespace std;

LectureHall::LectureHall(string pName, double pArea, int pDoors, Position pPosition, int pChairs) :
        Room(pName, pArea, pDoors, pPosition) {
    chairs = pChairs;
}

void LectureHall::printRoom() {
    cout << expand_self << "\tChairs: "  << chairs << endl;
}

int LectureHall::getChairs() const {
    return chairs;
}

void LectureHall::setChairs(int chairs) {
    this->chairs = chairs;
}

Lab.h

#ifndef LAB_H_
#define LAB_H_

#include "Room.h"

using namespace std;

class Lab: public Room {

        int computers;

    public:

        Lab(string pName, double pArea, int pDoors, Position pPosition, int pComputers);
        virtual void printRoom();
        virtual ~Lab() {
        }
        int getComputers() const;
        void setComputers(int computers);
        double getEquipmentDensity();
};

#endif /* LAB_H_ */

Lab.cpp

#include "Lab.h"

#include <iostream>
#include <string>

#include "Position.h"

using namespace std;
//=========================================================================
// CONSTRUCTOR
//=========================================================================
Lab::Lab(string pName, double pArea, int pDoors, Position pPosition, int pComputers) :
        Room(pName, pArea, pDoors, pPosition) {
    computers = pComputers;
}
void Lab::printRoom() {
    cout << expand_self << "\tComputers: " << computers << endl;
}
int Lab::getComputers() const {
    return computers;
}

void Lab::setComputers(int computers) {
    this->computers = computers;
}

double Lab::getEquipmentDensity() {
    return computers / area;
}

Client.h

#ifndef CLIENT_H_
#define CLIENT_H_

#include "RoomBookingSystem.h"

using namespace std;

class Client {

        RoomBookingSystem* rbs;

        int getRoomType();
        int getRoomByWhat();
        Room* getRoom(int type);
        template<typename T> T input(const string& s) const;

    public:

        Client();
        ~Client();
        void printChoices();
        void addRoom();
        void getMinimumRoomsofArea();
        void printRoom();
        void getSmallestLectureHallForGivenStudents();
        void getLeastEquipmentDensityLab();
        void getNearestRoom();
        void removeRoom();
        void bookUnbook();
};

#endif /* CLIENT_H_ */

Client.cpp

#include "Client.h"

#include <cstdlib>
#include <iostream>
#include <iterator>
#include <string>
#include <vector>

#include "Lab.h"
#include "LectureHall.h"

#define NAME 1
#define POSITION 2

#define BOOK 1
#define UNBOOK 2

#define CIN_IGNORE_MAX 512

using namespace std;

void Client::printChoices() {
    cout << "Press the corresponding keys for these operations:" << endl;
    cout << "1. Add Room/Lab/Lecture Hall." << endl;
    cout << "2. Remove Room/Lab/Lecture Hall." << endl;
    cout << "3. Book/Unbook a Room/Lab/Lecture Hall." << endl;
    cout << "4. Get minimum number of rooms to accommodate given number of persons." << endl;
    cout << "5. Get smallest lecture hall to accommodate given number of students." << endl;
    cout << "6. Get lab with least equipment density or change lab equipment quantities." << endl;
    cout << "7. Find a room nearest to a location." << endl;
    cout << "8. Print Room/Lab/Lecture Hall details." << endl;
    cout << "9. Exit." << endl;
}
Client::Client() {
    rbs = new RoomBookingSystem();
}
Client::~Client() {
    delete rbs;
}
int Client::getRoomType() {
    cout << ROOMS << ". Room." << endl;
    cout << LABS << ". Lab." << endl;
    cout << LECTURE_HALLS << ". Lecture Hall." << endl;
    int choice = -1;
    while ((choice = input<int>("Enter Choice:")) < ROOMS || choice > LECTURE_HALLS)
        cout << "Please enter valid option." << endl;
    return choice;
}

int Client::getRoomByWhat() {
    cout << "Choose:" << endl;
    cout << NAME << ". By Name." << endl;
    cout << POSITION << ". By Position." << endl;
    int choice = -1;
    while ((choice = input<int>("Enter Choice:")) < NAME || choice > POSITION)
        cout << "Please enter valid option." << endl;
    return choice;
}

void Client::addRoom() {
    int roomType = getRoomType();
    string name = input<string>("Enter Name: ");
    double area = input<double>("Enter Area: ");
    int doors = input<int>("Enter Doors: ");
    int x = input<int>("Enter Position X: ");
    int y = input<int>("Enter Position Y: ");
    switch (roomType) {
        case ROOMS:
            rbs->addRoom(name, area, doors, x, y);
            cout << "Room Added." << endl;
            break;

        case LABS: {
            int computers = input<int>("Enter Computers: ");
            rbs->addLab(name, area, doors, x, y, computers);
        }
            cout << "Lab Added." << endl;
            break;
        case LECTURE_HALLS: {
            int chairs = input<int>("Enter Chairs: ");
            rbs->addLectureHall(name, area, doors, x, y, chairs);
        }
            cout << "Lecture Hall Added." << endl;
            break;
    }
}

void Client::removeRoom() {
    int roomType = getRoomType();
    Room* r = getRoom(roomType);
    if (rbs->removeRoom(roomType, r))
        cout << "Room removed successfully." << endl;
    else
        cout << "Room not removed." << endl;
}

Room* Client::getRoom(int type) {
    switch (getRoomByWhat()) {
        case NAME: {
            string name = input<string>("Enter Name.");
            return rbs->getRoom(name, type);
        }
        case POSITION: {
            int x = input<int>("Enter Position X.");
            int y = input<int>("Enter Position Y.");
            return rbs->getRoom(x, y, type);
        }
    }
    return NULL;
}
void Client::bookUnbook() {
    Room* r = getRoom(getRoomType());
    cout << "This room is " << ((r->isBooked()) ? "booked" : "un-booked") << ". Do you want to " << ((r->isBooked()) ? "un-book" : "book") << " it?" << endl;
    char toggleBook = 'z';
    while (toggleBook != 'y' && toggleBook != 'n') {
        cout << "Enter.(y/n)" << endl;
        cin >> toggleBook;
    }
    if (toggleBook == 'y') {
        if (!r->isBooked()) {
            r->book();
        } else {
            r->unbook();
        }
        cout << "Room successfully " << ((r->isBooked()) ? "booked." : "un-booked.") << endl;
    } else {
        cout << "OK." << endl;
    }
}
void Client::getMinimumRoomsofArea() {
    double area;
    while ((area = input<double>("Enter Area(>0): ")) <= 0)
        ;
    vector<Room*> v = rbs->getMinimumRoomsOfArea(area);
    if (v.size() == 0) {
        cout << "Not Enough Rooms." << endl;
        return;
    }
    cout << "You may use these " << v.size() << " un-booked rooms." << endl;
    for (vector<Room*>::iterator it = v.begin(); it != v.end(); it++)
        (*it)->printRoom();
}
void Client::printRoom() {
    int choice = -1;
    while ((choice = input<int>("Enter 1 for specific Room/Lab/Lecture Hall and 2 for all.")) < 1 || choice > 2)
        ;
    switch (choice) {
        case 1: {
            int type = getRoomType();
            Room* r = getRoom(type);
            if (type == LABS) {
                ((Lab*) r)->printRoom();
            } else if (type == LECTURE_HALLS) {
                ((LectureHall*) r)->printRoom();
            } else {
                r->printRoom();
            }
            break;
        }
        case 2:
            rbs->printAll();
            break;
    }
}

void Client::getSmallestLectureHallForGivenStudents() {
    int students;
    while ((students = input<int>("Enter Students (>0): ")) <= 0)
        ;
    LectureHall* r = rbs->getSmallestLectureHallByStudents(students);
    if (r->getChairs() > 0) {
        cout << "The required Lecture Hall is:" << endl;
        r->printRoom();
    } else {
        cout << "No such Lecture Hall." << endl;
    }
}
void Client::getLeastEquipmentDensityLab() {
    int choice = -1;
    while ((choice = input<int>("Enter 1 to get the lab with least equipment density and 2 to increase equipment density of a lab.")) < 1 || choice > 2)
        ;
    switch (choice) {
        case 1: {
            Room* r = rbs->getLabByLeastEquipmentDensity();
            if (r->getArea() > 0) {
                r->printRoom();
            } else {
                cout << "No such Labs." << endl;
            }
        }
            break;
        case 2: {
            Lab* l = (Lab*) getRoom(LABS);
            cout << "The selected lab is:" << endl;
            l->printRoom();
            int newEqu = -1;
            while ((newEqu = input<int>("Enter the new Lab Equipments.")) < 0)
                ;
            l->setComputers(newEqu);
            break;
        }
    }
}
void Client::getNearestRoom() {
    int x = input<int>("Enter Position X:"), y = input<int>("Enter Position Y:");
    Room* r = rbs->getNearestRoom(x, y);
    if (r->getArea() > 0) {
        r->printRoom();
    } else {
        cout << "No such Labs." << endl;
    }
}

template<typename T> T Client::input(const string& s) const {
    T x;
    do {
        cout << s << endl;
        cin >> x;
        if (!cin) {
            cout << "Please enter valid option." << endl;
            cin.clear();
            cin.ignore(CIN_IGNORE_MAX, '\n');
        } else
            break;
    } while (true);
    return x;
}

int main(int argc, char **argv) {
    Client* client = new Client();
    while (true) {
        client->printChoices();
        int choice;
        while (!(cin >> choice) || choice < 1 || choice > 9) {
            cout << "Please choose from 1-9" << endl;
            cin.clear();
            cin.ignore(CIN_IGNORE_MAX, '\n');
        }
        switch (choice) {
            case 1:
                client->addRoom();
                break;
            case 2:
                client->removeRoom();
                break;
            case 3:
                client->bookUnbook();
                break;
            case 4:
                client->getMinimumRoomsofArea();
                break;
            case 5:
                client->getSmallestLectureHallForGivenStudents();
                break;
            case 6:
                client->getLeastEquipmentDensityLab();
                break;
            case 7:
                client->getNearestRoom();
                break;
            case 8:
                client->printRoom();
                break;
            case 9:
                delete client;
                cout << "Exited." << endl;
                exit(0);
                break;
        }
        char cont = 'z';
        while (cont != 'y' && cont != 'n') {
            cout << "Continue?(y/n)" << endl;
            cin >> cont;
        }
        if (cont == 'n') {
            cout << "Exited." << endl;
            delete client;
            exit(0);
            break;
        }

    }
}
\$\endgroup\$
1
  • \$\begingroup\$ Your room booking system class looks like God object \$\endgroup\$ Commented Sep 23, 2016 at 7:51

1 Answer 1

5
\$\begingroup\$

Lets start simple, I would drop the using namespace std; line from all of your files and use std::foo instead to avoid namespace pollution. Which could bite you later on or in future projects.

In addition, your #define statements such as #define ROOM_FILE_LOC "data/ROOM.txt" and #define LECTURE_HALLS 3 should be const variables. This just allows the compiler to type-check and the variables to be scoped. One exception to this is if you are concatenating #define strings, but I don't see that in the code.

In Client.cpp, I notice a lot of std::endl statements. Note that this usually means the output buffer is being flushed after every output statement. For example, if the following code was changed from this:

cout << "Press the corresponding keys for these operations:" << endl;
cout << "1. Add Room/Lab/Lecture Hall." << endl;
cout << "2. Remove Room/Lab/Lecture Hall." << endl;
cout << "3. Book/Unbook a Room/Lab/Lecture Hall." << endl;
cout << "4. Get minimum number of rooms to accommodate given number of persons." << endl;
cout << "5. Get smallest lecture hall to accommodate given number of students." << endl;
cout << "6. Get lab with least equipment density or change lab equipment quantities." << endl;
cout << "7. Find a room nearest to a location." << endl;
cout << "8. Print Room/Lab/Lecture Hall details." << endl;
cout << "9. Exit." << endl;

To this:

std::cout << "Press the corresponding keys for these operations:\n";
std::cout << "1. Add Room/Lab/Lecture Hall.\n";
std::cout << "2. Remove Room/Lab/Lecture Hall.\n";
std::cout << "3. Book/Unbook a Room/Lab/Lecture Hall.\n";
std::cout << "4. Get minimum number of rooms to accommodate given number of persons.\n";
std::cout << "5. Get smallest lecture hall to accommodate given number of students.\n";
std::cout << "6. Get lab with least equipment density or change lab equipment quantities.\n";
std::cout << "7. Find a room nearest to a location.\n";
std::cout << "8. Print Room/Lab/Lecture Hall details.\n";
std::cout << "9. Exit." << std::endl;

Only the last std::endl would flush the output buffer and display the above text all at once. This answer explains the output buffer extremely well.

In a few of your classes, I noticed that std::cout is used in the Foo::printBar() functions. I highly recommend decoupling this dependency on the terminal. Think about what happens if this application were to move from a command line interface to a graphical one. These functions would cause a headache.

Instead, make sure the only thing that can print to or read from the terminal is the Client class. Instead, refactor the void Foo::printBar() functions into string Foo::toString() functions that return a string that your Client class can use to determine how to display the information instead.

The RoomBookingSystem seems to be a God Object as pointed out by Olzhas. To fix this, remember the single responsibility principle. Each class should do one thing well. As is, the RoomBookingSystem manages rooms, prints output, and saves & loads files.

For example, lets do the save/load functionality. The first step is to take the same approach as we did with the Client class. That is to remove the dependency on files. Think about what would happen if we wanted to change the way the data was stored from files to a database instead. The way the information is stored should be hidden to the RoomBookingSystem, since the RoomBookingSystem doesn't care about where it gets its information from.

This change would involve moving the saveRooms(...) and read(...) functions to their own file, defining an interface for the new class to implement (with functions such as readRoomDetails() and getLectureHallDetails(). Although something to keep in mind is what happens if we want to implement something new, that isn't a lab or lecture hall. This depends on what the design goals are for the application of how 'hard coded' you want to make this interface (that can be implemented by the file method or database method).

Keep common code together. This keeps the amount of things we need to track in our mind much smaller. For instance:

RoomBookingSystem::RoomBookingSystem() {
        vector<vector<string> > roomDetails = read(ROOM_FILE_LOC, 5);
        vector<vector<string> > labDetails = read(LAB_FILE_LOC, 6);
        vector<vector<string> > lectureHallDetails = read(LECTURE_HALL_FILE_LOC, 6);
        vector<vector<string> > roomBookDetails = read(ROOM_BOOK_LOC, 1);
        vector<vector<string> > labBookDetails = read(LAB_BOOK_LOC, 1);
        vector<vector<string> > lectureHallBookDetails = read(LECTURE_HALL_BOOK_LOC, 1);
        parse(roomDetails, ROOMS);
        parse(labDetails, LABS);
        parse(lectureHallDetails, LECTURE_HALLS);
        loadstate(roomBookDetails, ROOMS);
        loadstate(labBookDetails, LABS);
        loadstate(lectureHallBookDetails, LECTURE_HALLS);
}

Could be changed to:

RoomBookingSystem::RoomBookingSystem() {
    vector<vector<string> > roomDetails = read(ROOM_FILE_LOC, 5);
    parse(roomDetails, ROOMS);

    vector<vector<string> > roomBookDetails = read(ROOM_BOOK_LOC, 1);
    loadstate(roomBookDetails, ROOMS);

    vector<vector<string> > labDetails = read(LAB_FILE_LOC, 6);
    parse(labDetails, LABS);

    vector<vector<string> > labBookDetails = read(LAB_BOOK_LOC, 1);
    loadstate(labBookDetails, LABS);

    vector<vector<string> > lectureHallDetails = read(LECTURE_HALL_FILE_LOC, 6);
    parse(lectureHallDetails, LECTURE_HALLS);

    vector<vector<string> > lectureHallBookDetails = read(LECTURE_HALL_BOOK_LOC, 1);
    loadstate(lectureHallBookDetails, LECTURE_HALLS);

}

Lastly, I would create more objects. For instance, vector<vector<string>> isn't very clear as to what it is storing and seems to be used in multiple places around the application. To clean it up, we could create an object such as RoomDetails and BookStatus and give them methods to access and modify their data. This avoids the complicated need for lines such as:

case LABS:
    labss.push_back(new Lab(expand_it, atoi((*it)[5].c_str())));
    break;

Or

vector<Room*>* vec = getList(type);
for (unsigned i = 0; i < vec->size(); i++) {
        ((*vec)[i])->setBooked(atoi(bookstatus[i][0].c_str()) == 1);
}

since they will be abstracted away in their own classes where the implementation can be freely changed without RoomBookingSystem knowing or caring about it.

This seems like a complicated system, take your time and care when refactoring and remember to always test often. Using unit testing can greatly decrease the chance of bugs entering the system after major modifications.

\$\endgroup\$
2
  • \$\begingroup\$ Could you explain me what you mean by "use std::foo instead"? \$\endgroup\$
    – Kodnot
    Commented Sep 23, 2016 at 18:28
  • \$\begingroup\$ So the line using namespace std; includes all of the std namespace (which happens to be quite large), when most applications only need a small subset of it. One way to accomplish this is to explicitly reference the namespace in the function call itself. For instance, std::cout << "Hello!" << std::endl;. \$\endgroup\$
    – JD.B
    Commented Sep 23, 2016 at 18:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.