patterncppMinor

Room Booking System

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

bookingsystemroom

Problem

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
#include
#include

#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 > roomDetails, int type);
void loadstate(vector > bookstatus, int type);
vector* getList(int type);
vector* getConstList(int type) const;
template Room* getRoom(int type, const T matcher) const;
template void print(vector& vec, const string& pDescriptor);
struct RoomComparator {
bool operator()(const int i, const int j);
};
void saveRooms(vector& vec, const char loc, const char bookloc);
vector > 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, c

Solution

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 > roomDetails = read(ROOM_FILE_LOC, 5);
        vector > labDetails = read(LAB_FILE_LOC, 6);
        vector > lectureHallDetails = read(LECTURE_HALL_FILE_LOC, 6);
        vector > roomBookDetails = read(ROOM_BOOK_LOC, 1);
        vector > labBookDetails = read(LAB_BOOK_LOC, 1);
        vector > 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 > roomDetails = read(ROOM_FILE_LOC, 5);
parse(roomDetails, ROOMS);

Code Snippets

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;

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;

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() {
    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);

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

Context

StackExchange Code Review Q#142174, answer score: 5

Revisions (0)

No revisions yet.