1
\$\begingroup\$

I'm a coding newbie (and despite what my user name may imply I am far from a pro), and I'm trying to write my own text-based adventure game. I have two questions.

First, I want to implement an Object class. These Objects have names and descriptions and can be placed in rooms, as well as picked up and carried around by the player. What's messing me up is that these Objects are supposed to know what room they were originally in, their "homeroom" so to speak.

I'm not sure how to let each Room know that they have Objects placed within them. Everything that I've tried to do has failed to compile.

I've tried to include Room r as a private variable in Object.cpp and include Room to the Object constructor.

Object::Object(string name, string description, Room *r)
{
    name_ = name; 
    description_ = description; 
    r_ = r; //assume r_ is a private variable

}

Secondly, regarding pointers... This assignment specifies that I must have a vector of pointers of Objects. Would it look like this?

vector<Object*>objectsInRoom; 

In main.cpp, I also need a vector of Objects. Is the vector inside the Room class keeping track of Objects in each Room? And is the vector in main.cpp keeping track of all the objects the player carries. Why must the room class have a vector of pointer of Objects? Would not having a vector of Objects suffice?

(I apologize if this sounds vague; this game is based off of an assignment that can be found here. If you scroll down to the "Extra Credit" portion and go to the first paragraph block marked 10 points, you'll find a much more extensive explanation that I tried to condense above.)

main.cpp

#include "Room.h"
#include "Object.h"
#include <iostream>
#include <iomanip>
#include <fstream>
#include <cstdlib>

using namespace std;

void menu();
void look(Room* current_room);
void exits(Room* current_room);
void add(Room* current, vector<Room*>& allRooms, vector<string>&roomConnections, bool mode);
void connect(Room* current, vector<Room*> allRooms, vector<string>&roomConnections, bool mode);
void rename(Room* current);
void description(Room* current);
bool isConnected(Room*current, Direction dir); 
void jump(Room* &current, vector<Room*> allRooms); 
void save(Room* current, vector<Room*> allRooms, vector<string>roomConnections);
void load(Room*&current, vector<Room*>& allRooms, vector<string>&roomConnections); 
void initialize(vector<string>&roomConnections, int size); 
Direction char2Direction(char c); // convert a char value ('N','S','E','W')
                                // to an enum value NORTH, SOUTH, etc
char digit2letter(char n);
string letter2digit(char l); 
bool allFourConnected(Room*current); 

int main()
{
    // initialize
    Room* current_room = new Room();
    //Object current_object = Object(); 
    //cout << current_object.getName() << endl; 
    vector<Room*> allRooms;
    vector<string>roomConnections;
    initialize(roomConnections, 25); 
    vector<Object>allObjects; 
    allRooms.push_back(current_room);
    Object currentObject; 
    string userInput; 
    char choice;
    bool builderMode; 

    cout << "Builder Mode on or off? Press 1 for ON, 0 for OFF." << endl; 
    cin >> builderMode; 

    //show name and description of current_room
    look(current_room); 
    // menu loop
    do
    {
        cout << "Enter a command (? to list): ";
        cin>>userInput;
        choice = tolower(userInput[0]); 
        if (userInput.compare("exits") == 0)
            choice = 'x'; 
        else if (userInput.compare("save") == 0)
            choice = 'z';
        else if (userInput.compare("load") == 0)
            choice = 'b';

        switch(choice)
        {
            case 'n':
                if (current_room->north()!= NULL)
                    current_room = current_room->north();
                else
                    cout<<"You cannot go that way"<<endl;
                break;
            case 'e':
                if (current_room->east()!= NULL)
                    current_room = current_room->east();
                else
                    cout<<"You cannot go that way"<<endl;
                break;
            case 's':
                if (current_room->south()!= NULL)
                    current_room = current_room->south();
                else
                    cout<<"You cannot go that way"<<endl;
                break;
            case 'w':
                if (current_room->west()!= NULL)
                    current_room = current_room->west();
                else
                    cout<<"You cannot go that way"<<endl;
                break;
            case 'l':
                look(current_room); break;
            case 'x':
                exits(current_room);break;
            case 'c':
                connect(current_room, allRooms, roomConnections, builderMode); break;
            case 'r':
                rename(current_room); break;
            case 'd':
                description(current_room); break;
            case 'a':
                add(current_room, allRooms, roomConnections, builderMode); break;
            case 'j': 
                jump(current_room, allRooms); break; 
            case 'z': 
                save(current_room, allRooms, roomConnections); break;
            case 'b': 
                load(current_room, allRooms, roomConnections); break;
            case '?':
                menu(); break;
            default:
                cout<<"invalid command"<<endl;
        }
    }while(choice != 'q');



    // cleanup before quitting

    return 0;
}

// Show the name and description of a room */
void look(Room* r)
{
    cout << r->name() << endl; 
    cout << r->description() << endl; 

}

// add a new room, connected to the current room
//  Parameters
//     current -- the current room we are in
//     allRooms-- a vector of pointers to (addresses of) all the rooms in the game
//  Precondition
//     current refers to an existing room
//  Postcondition
//    if possible (current must have a free exit)
//     a) a new room has been added off one of the exits of current
//     b) the address of the new room has also been added to the allRooms vector
void add(Room* current, vector<Room*>& allRooms, vector<string>&roomConnections, bool mode)
{
    string strExit, strTo, eDigit, rDigit, tDigit; Direction exit, to; 
    int index, dirExit, dirTo; 
    Room* temp; bool flag; 
    if (mode)
    {
        if (allFourConnected(current))
            cout << "All four exits are connected!" << endl;
        else
        {


            //create a new room 
            temp = new Room();

            //determine the direction and validity of current's exit to the new room
            do
            {
                cout << "Connect new room in which direction (N, S, E, W)? ";
                cin >> strExit;
                exit = char2Direction(strExit[0]);
                flag = isConnected(current, exit);

                if (flag)
                    cout << "That exit is already connected!" << endl;
            } while (flag);

            //determine the direction of the new room's exit to current
            cout << "Which exit of the new room leads to this room? ";
            cin >> strTo;
            to = char2Direction(strTo[0]);


            //connecting current to new room
            current->connect(exit, temp, to);


            eDigit = letter2digit(strExit[0]);
            rDigit = to_string(allRooms.size());
            tDigit = letter2digit(strTo[0]);

            dirExit = eDigit[0] - '0';
            cout << "dirExit = " << dirExit << " ";

            //determing where to place the above digits 
            index = (allRooms.size() - 1) * 4 + dirExit;
            roomConnections[index] = eDigit + rDigit + tDigit;

            cout << "index = " << index << endl;
            cout << "roomConnections[index] = " << roomConnections[index] << endl;

            //add the address of the new room to allRooms
            allRooms.push_back(temp);
            /*
            cout << allRooms.size() << " " << endl;
            cout << "showing roomConnections = ";
            for (int i = 0; i < 25; i++)
                cout << roomConnections[i] << " ";
            */
            cout << "Room is connected ok" << endl;
        }
    }
    else
        cout << "Builder mode is OFF" << endl; 
}

// add a new connection between current room and an already existing room.
//  Parameters
//     current -- the current room we are in
//     allRooms-- a vector of pointers to (addresses of) all the rooms in the game
//  Precondition
//     current refers to an existing room
//  Postcondition
//    if possible (both rooms must have a spare exit to link them together)
//     current is now linked to another room in the game
//
void connect(Room* current, vector<Room*> allRooms, vector<string>&roomConnections, bool mode)
{
    string strExit, strTo, eDigit, rDigit, tDigit; Direction exit, to; bool flag; int cellNum, dirExit, roomNum, index;

    if (mode)
    {
        //cout << "name of room = " << current->name() << endl; 
        //find the index of current in allRooms
        for (int i = 0; i < allRooms.size(); i++)
        {
            if (current == allRooms[i]) //comparing addresses
            {
                roomNum = i;
            }
        }

        //determine the direction of current's exit to existing room it's connecting to
        //determine whether exit is valid 
        if (allFourConnected(current))
            cout << "All four rooms are connected!" << endl;
        else
        {
            do
            {
                cout << "Connect new room in which direction (N, S, E, W)? ";
                cin >> strExit;
                exit = char2Direction(strExit[0]);
                flag = isConnected(current, exit);
                cout << "flag = " << flag << endl;
                if (flag) //if exit is already connected
                    cout << "That exit is already connected!" << endl;
            } while (flag);


            //determine what the existing room current room wants to connect to 
            //determine whether room number is valid 
            cout << "Connect " << current->name() << " to which room (enter #)? " << endl;

            do
            {
                for (int i = 0; i < allRooms.size(); i++)
                    cout << "      " << i << ". " << allRooms[i]->name() << endl;

                cin >> cellNum;
            } while (cellNum > allRooms.size() - 1);


            //determine the direction of existing room's exit to current room
            //determine whether exit is valid 
            do {
                cout << "Which exit of the " << allRooms[cellNum]->name() << " leads to " << current->name() << "? ";
                cin >> strTo;
                to = char2Direction(strTo[0]);
                flag = isConnected(allRooms[cellNum], to);

                if (flag)
                    cout << "That exit is already connected" << endl;
            } while (flag);


            //connecting the two rooms
            current->connect(exit, allRooms[cellNum], to);
            eDigit = letter2digit(strExit[0]);
            rDigit = to_string(cellNum);
            tDigit = letter2digit(strTo[0]);

            //cout << "eDigit = " << eDigit << " rDigit = " << rDigit << " tDigit = " << tDigit << endl; 
            dirExit = eDigit[0] - '0';

            //cout << "roomNum = " << roomNum << " "; 

            //determing where to place the above digits 
            index = roomNum * 4 + dirExit;

            //cout << "index = " << index << endl; 
            roomConnections[index] = eDigit + rDigit + tDigit;
            //cout << "roomConnections[index] = " << roomConnections[index] << endl;
            cout << "Room is connected ok" << endl;
        }
    }
    else
        cout << "Builder mode is OFF!" << endl; 
}

// change the name of a room
void rename(Room* current)
{
    string name; 
    cout << "Enter a new name for this room and hit <enter>: "; 
    cin >> name; 
    current->set_name(name); 
}

// change the description of a room
void description(Room* current)
{
    string text;
    cout<<"Enter a new description for this room and hit <enter> ";
    cin.ignore(1,'\n'); // ignore any leading line breaks
    getline(cin,text);  // read one line of text 
    current->set_description(text); //set description of currernt room to text we just read 
}

// for each exit from room r,
//   show the name of the room it connects to
//   or say "Nothing" if the link is null
void exits(Room* r)
{ //setw(15)
    cout << "    North-->"; 
    if (r->north() == NULL)
        cout << " Nothing";
    else
        cout << " " << r->north()->name(); 
    cout << endl; 

    cout << "    East -->"; 
    if (r->east() == NULL)
        cout << " Nothing";
    else
        cout << " " << r->east()->name(); 
    cout << endl; 

    cout << "    South-->"; 
    if (r->south() == NULL)
        cout << " Nothing";
    else
        cout << " " << r->south()->name(); 
    cout << endl; 

    cout << "    West -->";
    if (r->west() == NULL)
        cout << " Nothing";
    else
        cout << " " << r->west()->name();
    cout << endl; 
}
void menu()
{
    cout<<"Please choose from the following: "<<endl;
    cout<<"     n or north --- move north (if possible)"<<endl;
    cout<<"      e or east --- move east (if possible)"<<endl;
    cout<<"     s or south --- move south (if possible)"<<endl;
    cout<<"      w or west --- move at west (if possible)"<<endl;
    cout<<"      l or look --- look at room"<<endl;
    cout<<"     x or exits --- show room exits"<<endl;
    cout<<"   c or connect --- connect this room to another (already made) room"<<endl;
    cout<<"    r or rename --- rename this room"<<endl;
    cout<<"      d or desc --- change description for this room"<<endl;
    cout<<"       a or add --- add new room"<<endl;
    cout<<"      j or jump --- jump to new room" << endl; 
    cout<<"      z or save --- save rooms and connections" << endl; 
    cout<<"      b or load --- load saved rooms and connections" << endl;
    cout<<"      q or quit --- exit program"<<endl;
    cout<<"              ? --- show commands (DUH)"<<endl;
}


// convert a char direction: 'N','S','E','W'
// into a Direction enum: NORTH, SOUTH, EAST, WEST
Direction char2Direction(char c)
{
    /*
    if (c == 'N')
        cout << "You're going to return NORTH!" << endl; 
    if (c == 'E')
        cout << "You're going to return 'EAST'!" << endl; 
    */
    switch(c)
    {
        case 'N': return NORTH;
        case 'S': return SOUTH;
        case 'E': return EAST;
        case 'W': return WEST;
        default: return NORTH;
    }
}

char digit2letter(char n)
{
    switch (n)
    {
        case '0': return 'N';
        case '1': return 'E'; 
        case '2': return 'S'; 
        case '3': return 'W'; 
        default: return 'N'; 

    }

}

string letter2digit(char l)
{
    switch (l)
    {
    case 'N': return "0";
    case 'E': return "1";
    case 'S': return "2";
    case 'W': return "3";
    default: return "0";

    }
}

//return false if current->direction() == NULL, meaning you can connect
//return true if current->direction() != NULL, meaning there's already a connection 
bool isConnected(Room*current, Direction dir)
{

    if (dir == 0)
    {
        if (current->north() != NULL) 
            return true;
        else
            return false;
    }

    if (dir == 1)
    {
        if (current->east() != NULL)
            return true;
        else
            return false;
    }

    if (dir == 2)
    {
        if (current->south() != NULL)
            return true;
        else
            return false;
    }

    if (dir == 3)
    {
        if (current->west() != NULL)
            return true;
        else
            return false;
    }
}
//return true if all four exits are connected, false otherwise 
bool allFourConnected(Room* current)
{
    if (current->north() != NULL & current->east() != NULL
        & current->south() != NULL & current->west() != NULL)
        return true; 
    return false;
}

void jump(Room* &current, vector<Room*> allRooms)
{
    bool foundRoom = false; int cellNum; 
    cout << "Which room do you want to jump to (enter a number)? " << endl; 

    do
    {
        for (int i = 0; i < allRooms.size(); i++)
            cout << "      " << i << ". " << allRooms[i]->name() << endl;

        cin >> cellNum;
    } while (cellNum > allRooms.size() - 1);

    current = allRooms[cellNum]; 
}

void save(Room* current, vector<Room*> allRooms, vector<string>roomConnections)
{ 
    int cellNum = 0; 
    ofstream gameFile("savedGame.txt");
    Room* last = current; //player has to be in the same room she was when she saved the game 

    //write the room number player is in when she saves the game
    for (int i = 0; i < allRooms.size(); i++)
    {
        if (last == allRooms[i])
            cellNum = i;
    }

    gameFile << cellNum << endl;

    //write in total number of rooms
    gameFile << allRooms.size() << endl; 
    cellNum = 0; 
    for (int i = 0; i < allRooms.size(); i++)
    {
        current = allRooms[i]; 
        //write name 
        gameFile << current->name() << endl;

        //write description 
        gameFile << current->description() << endl;
        string exits = "";

        for (int k = 0; k<4; k++)
        {
            gameFile << roomConnections[cellNum];
            cellNum++;
        }
        gameFile << endl; 
        //specify the rooms the exits are connected to 

    }   

}

void load(Room* &current, vector<Room*>&allRooms, vector<string>&roomConnections)
{

    ifstream gameFile("savedGame.txt");
    string name, description, currentDir, tempDir, tempExits, currentExits;
    Room*  temp; int k = 0, size = 1, totalRooms = 0;
    Direction exit, to; int cellNum; 


    gameFile >> cellNum; 
    //cout << "cellNum = " << cellNum << endl; 

    gameFile >> totalRooms; 
    //cout << totalRooms << endl;
    for (int k = 0; k<totalRooms; k++)//three lines of code for each room  
    {

        //set name
        gameFile >> name;
        //cout << "name: " << name << endl;
        current->set_name(name);
        gameFile.ignore(); 

        //set description
        getline(gameFile, description);
        //cout << "description: " << description << endl;
        current->set_description(description);

        //put roomConnections in a string
        getline(gameFile, currentExits);


        //sort out the exit connections; those with none and those with 
        //add them to roomConnections- an array of strings that keeps track of all the exits 
        for (int i = 4 * k; i < 4 * k + 4; i++)
        {
            if (currentExits.substr(0, 2).compare("-1") != 0) //there is a connection 
            {
                //cout << "HELLO    "; 
                roomConnections[i] = currentExits.substr(0, 3);
                //cout << "roomConnections[i] =" << roomConnections[i] << endl; 
                currentExits = currentExits.substr(3, currentExits.length() - 3);
                //cout << currentExits << endl; 
            }
            else
            {
                //cout << "GOODBYE    "; 
                roomConnections[i] = currentExits.substr(0, 2);
                //cout << "roomConnections[i] =" << roomConnections[i] << endl;
                currentExits = currentExits.substr(2, currentExits.length() - 2);
                //cout << currentExits << endl; 
            }
        }

        //cout << "totalRooms = " << totalRooms << endl; 

        if (!(k == 0))
            allRooms.push_back(current);
        current = new Room();

    }


    /*
    for (int i = 0; i < totalRooms*4; i++) //each room has 4 possible connections
        cout << roomConnections[i] << " ";
    cout << endl;

    for (int i = 0; i < allRooms.size(); i++)
        cout << i << " " << allRooms[i]->name() << endl; 

    */
    int roomCIndex = 0; 
    int roomAIndex = 0; 


    for (int i = 0; i < totalRooms*4; i++)
    {
        //if there is an exit connected to another room
        if (roomConnections[roomCIndex].substr(0, 2).compare( "-1"))
        {

            current = new Room(); temp = new Room();
            current = allRooms[roomAIndex];
            //cout << current->name() << ", ";

            //convert -exit- char to Direction 
            string digits = roomConnections[i];
            char exitChar = digit2letter(digits[0]);
            Direction exit = char2Direction(exitChar);
            //cout << "exit = " << exit << ", "; 

            //retrieve room you are connecting to 
            int cellNum = digits[1] - '0';
            temp = allRooms[cellNum];
            //cout << temp->name() << ", ";

            //convert -to- char to Direction 
            char toChar = digit2letter(digits[2]);
            Direction to = char2Direction(toChar);
            //cout << "to = " << to << endl;
            current->connect(exit, temp, to);

            //move index to next exit
            roomCIndex++;  
        }
        else //if there is not an exit connected
        {
            //cout << "GOODBYE" << endl;

            //move index to next exit 
            roomCIndex++; 
        }

        if ((i+1) % 4 == 0)
        {
            roomAIndex++; 
        }
    }

    //move to the room she was in when she saved the game 
    current = allRooms[cellNum]; 

}

void initialize(vector<string>&roomConnections, int size)
{
    for (int k = 0; k < size; k++)
        roomConnections.push_back("-1"); 
}

room.cpp

// Room.cpp: implementation of the Room class.
//
//////////////////////////////////////////////////////////////////////

#include "Room.h"


//////////////////////////////////////////////////////////////////////
// Construction/Destruction
//////////////////////////////////////////////////////////////////////

Room::Room()
{
    name_ = "The void";
    description_ = "There is nothing but blackness in every direction.";
    int i;
    for(i = 0; i < 4; i++) // set all exits to "closed"
        exits_.push_back(NULL);


}

Room::Room(string name, string desc)
{
    name_ = name;
    description_ = desc;
    int i;
    for(i = 0; i < 4; i++) // set all exits to "closed"
        exits_.push_back(NULL);


}

Room::~Room()
{
    cout << "Destroying: " << name_ << endl; 
    // make sure all exits to this room are
    // destroyed so that no one can try to enter
    // this room from another location
    if(exits_[NORTH] != NULL)
        disconnect(NORTH);
    if(exits_[EAST] != NULL)
        disconnect(EAST);
    if(exits_[SOUTH] != NULL)
        disconnect(SOUTH);
    if(exits_[WEST] != NULL)
        disconnect(WEST);
}

// --- inspectors ---
Room * Room::north() const
{
    return exits_[NORTH];
}

Room * Room::south() const
{
    return exits_[SOUTH];
}

Room * Room::east() const
{
    return exits_[EAST];
}

Room * Room::west() const
{
    return exits_[WEST];
}


string Room::name() const
{
    return name_;
}

string Room::description() const
{
    return description_;
}

/*
vector<Object> Room::object() const
{
    return roomObjects; 
}
*/

// --- mutators ---
void Room::set_name(string n)
{
    name_ = n;
}

void Room::set_description(string d)
{
    description_ = d;
}

/*
void Room::set_object(Object o)
{
    allObjects.push_back(o); 
}
*/
// --- facilitators ---
bool Room::connect(Direction exit, Room *r, Direction to)
{
    // check that both exits are free
    if (exits_[exit] != NULL or r->exits_[to] != NULL)
        return false;
    // make connection
    exits_[exit] = r;
    r->exits_[to] = this;
    return true;
}

// --- private methods ---

void Room::disconnect(Direction d)
{
    // disconnects ALL exits from another
    // room to this one.  It's sloppy, but
    // that's OK.
    Room * other_room;
    other_room = exits_[d];
    int i;
    for(i = 0; i < 4; i++)  {
        if (other_room->exits_[i] == this)
            other_room->exits_[i] = NULL;
    }
}

// --- operators ---

ostream & operator<<(ostream & out, const Room & r) {
    out << r.name() << endl;
    out << r.description() << endl;
    return out;
}

Object.cpp

#include "Object.h"; 

Object::Object()
{
    name_ = "Object"; 
    description_ = "The object lies in the room"; 

}

Object::Object(string name, string description)
{
    name_ = name; 
    description_ = description; 


}

Object.h

#ifndef Object_H
#define Object_H

#include<string>
#include<ciso646>  
#include<vector>
#include<iostream>
#include <istream>
#include "Room.h"
using namespace std;

class Object
{
public: 
    //constructors
    Object(); 
    Object(string name, string description, Room* home_room); 


    //getters
    string getName(); 
    string getDescription(); 

    //setters
    void setName(string name); 
    void setDescription(string descr); 

    Object create(); 

private:
    string name_; 
    string description_; 

};
#endif

EDIT Below are my compiling errors when I compiled the above code (about pointers).

Blockquote

\$\endgroup\$

closed as off-topic by Sᴀᴍ Onᴇᴌᴀ, t3chb0t, Mast, Vogel612, Toby Speight Jan 15 '18 at 9:18

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. After the question has been edited to contain working code, we will consider reopening it." – Sᴀᴍ Onᴇᴌᴀ, t3chb0t, Mast, Vogel612, Toby Speight
If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ Your "Room.h" won't compile for me, due to the #ifndef (AFX_...) line. The parens are invalid according to g++ for ifndef. Based on the comment accompanying the #endif I reverted it to #if !defined(AFX_...) and got it working, but this seems like a bug. \$\endgroup\$ – Austin Hastings Jan 12 '18 at 9:25
  • \$\begingroup\$ Also, can you include your Object.h file? \$\endgroup\$ – Austin Hastings Jan 12 '18 at 9:27
  • 1
    \$\begingroup\$ Sure, included it:) \$\endgroup\$ – probeginner Jan 12 '18 at 9:31
  • 1
    \$\begingroup\$ Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. After the question has been edited to contain working code, we will consider reopening it. Please take a look at the help center. \$\endgroup\$ – Mast Jan 14 '18 at 17:00
  • 1
    \$\begingroup\$ It's riddled with easy to fix syntax errors. \$\endgroup\$ – Mast Jan 14 '18 at 17:01
1
\$\begingroup\$

Problems in Room.*

Your Room.h and Room.cpp have some serious issues. I think your instructors have gotten somewhat lazy. I'd suggest that you invite them to submit their code (Room.h and Room.cpp and whatever else they have written) to this site for review.

First, there is the problem that this file won't compile under g++:

$ g++ Room.cpp
In file included from Room.cpp:5:
./Room.h:5:9: error: macro name must be an identifier
#ifndef (AFX_ROOM_H__7C105847_EA72_4F3E_8758_2D94DD05D583__INCLUDED_)

At the end of the file, there is a comment #endif // !defined(AFX_ROOM_H__7C105847_EA72_4F3E_8758_2D94DD05D583__INCLUDED_)

I suspect that the #ifndef was once a #if !defined(...) that worked. I changed it in my version.

Next, the author(s) fell victim to one of the classic blunders! The most famous is "Never get involved in a land war in Asia." But only slightly less well known is this: "Never go in against a Sicilian when death is on the line!" And finally, we have the C++ coder's mantra:

Never put using namespace std; in a header file!

Which sadly they did, at line 13.

I commented out this line, and inserted these three lines instead:

using std::ostream;
using std::string;
using std::vector;

I then added this line to Room.cpp:

ostream & operator<<(ostream & out, const Room & r) {

    using std::endl; // <-- Added this line

Which got the last of the std:: symbols actually used by Room.

Problems in Object.*

I found the same problem in Object.h - the using namespace std;. So I deleted it. A similar solution applies.

And at this point we come to an impasse, because your Object.cpp won't compile:

$ g++ Object.cpp
Object.cpp:10:9: error: out-of-line definition of 'Object' does not match any
      declaration in 'Object'
Object::Object(string name, string description)
        ^~~~~~
1 error generated.

You mentioned that you were trying to get this working, so I'll give you a pass on it for now.

Problems in main.cpp

I switched to compiling main.cpp and found one warning:

$ g++ main.cpp
main.cpp:455:1: warning: control may reach end of non-void function [-Wreturn-type]
}
^
1 warning generated.

The function in question is this one:

bool isConnected(Room*current, Direction dir)
{

    if (dir == 0)
    {
        if (current->north() != NULL)
            return true;
        else
            return false;
    }

Here's a pro-tip: your Direction type is an enum. So you want to exhaustively handle all the values. That means using a switch statement. (You're doing this for a class, so I don't know if you have learned about switch or not. If not, skip this I guess.)

Instead of writing a long sequence of if/else if/else statements, put everything into a switch table. Another pro-tip: when you are using an if-statement to decide whether to return true or false, just convert the conditional expression of the if-statement into a boolean expression, and return that.

bool
isConnected(Room * room, Direction dir)
{
    switch (dir) {
    case NORTH: return room->north() != NULL;  // Returns true / false
    case EAST:  return room->east()  != NULL;
    case SOUTH: return room->south() != NULL;
    case WEST:  return room->west()  != NULL;
    default:
        // impossible! Throw an exception or something.
        return false;
    }
}

Note that in this switch, the default isn't really needed, since I've spelled out all the possible values. But it doesn't hurt to handle strange cases, since there's always some idiot who will cast an integer to an enum, or something. (Probably better to throw an exception, really, but I don't know if you've gotten that far.)

Back to Object.cpp

At this point, I can't get main.cpp to link because of Object.cpp so let's turn our attention to that.

First, I have serious questions about your statement, "these Objects are supposed to know what room they were originally in, their "homeroom" so to speak."

I looked in your homework assignment, and I see the line there about knowing what room they belong in. I just don't understand why. The only use for this is in one of the following credit portions, where objects that are homing will return to their home room when dropped. IMO, this is the kind of thing that should be implemented in the homing objects, not built in to every single object type.

Regardless, consider this: an object has a home room. It always has a home room. The home room cannot be changed.

What does that sound like to you? Because to me, it sounds like a reference, not a pointer. But supposedly this exercise is about pointers, so we'll do it with pointers, following WooWapDaBug's suggestion:

Object::Object(string name, string description, Room* home):
    name_(name),
    desc_(description),
    home_(home)
{
}

Memory Ownership in C++

One of the most important concepts in C++ when dealing with pointers is the idea of ownership. Some terms:

  • The return value of new (or malloc) is called a raw pointer. This is simply the memory address of an object, without any tricky "wrapper" around it.

  • Raw pointers don't have any semantics. A raw pointer is just a bunch of bits with a data-only type. The type is "pointer to X" (for some X). There are no methods, no constraints, no nothing.

  • It's amazingly easy to make a mistake when dealing with raw pointers. Years of stories on the evening news about another security vulnerability being discovered - that's how amazingly easy it is.

  • To combat this, C++ has various kinds of "smart pointers." These are template-based types that have some sort of associated ownership semantics attached to them.

  • If you haven't learned about smart pointers, and you don't have some sort of template (mental template, not C++ template) for how to use them in your mind, then don't try to crowbar them in now.

  • Instead, pick a DAMN SIMPLE rule for ownership. For this simple program, my suggestion is this: keep a global "master list" of things. Only delete a thing when you remove it from the master list. Only remove something from the master list when you delete it. Never delete using any other pointer. Ensure that every single object is on the master list.

(Please note: there are lots of ways to do ownership. There are owned pointers, shared pointers, reference counting pointers, weak pointers, garbage collected memory, and probably a dozen other ways. I'm giving you this suggestion to help you solve this one problem at this particular time. Nothing more.)

Enumerating Objects

Several of the extra credit items require or would benefit from having a number associated with the rooms and/or objects in the game. The load/save assignment practically requires it.

There is no requirement to support deleting rooms or objects that I can see. It seems reasonable then to attach an integer id to each object, and let the class manage them.

Similarly, it seems reasonable to have the class maintain the master lists of objects (mentioned above). The only time you would delete objects would be during the shutdown process.

It's worth pointing out that the index of an object in the master list is not likely to be a good id, since objects can be dropped in different rooms. Better to track the highest id assigned, and then give new objects an id that's one higher.

User class

Once you start adding support for picking up and dropping objects, you might as well just create a User class. I think you'll find that much of the code you write for User winds up moving into the Mobile class mentioned in the assignment, and then User is a subclass of Mobile. Or perhaps they both share a common parent class, since the user won't be loaded or saved (I think?).

\$\endgroup\$
  • \$\begingroup\$ Thank you for this amazing feedback. I'm not advanced enough of a coder to grasp everything you just said right this second, but the wealth of information you provided me will definitely help me going forward. I'll definitely be doing a lot of research. Right now, I'm just trying to get my Object.cpp to compile. I've actually tried variations of what you suggested, but it's not compiling properly. I've edited my post to link to an image of my compiling errors. One last question: do you have a c++ beginner book that you'd recommend? \$\endgroup\$ – probeginner Jan 12 '18 at 11:13
  • 1
    \$\begingroup\$ you don't include your custom version of room.h. But I suspect you're caught in a dependency loop. Room wants to have a vector of Objects, and Object wants to have a pointer to Room. In this case, you can solve it by not trying to include Room.h in Object.h, and just declaring class Room; so that you can later say Room * home. \$\endgroup\$ – Austin Hastings Jan 12 '18 at 11:18
  • 1
    \$\begingroup\$ I don't have any C++ books to recommend, sorry. I've picked up what C++ I know by osmosis over many years, so even the old classics don't seem relevant any more. The class Room; works because Object only needs a pointer to a Room, and all pointers are basically the same size under the hood. So you can tell the compiler "there's a class called Room, I'll fill you in later" and then tell it "this function takes a pointer to Room" and the compiler is okay with that. But if you try to access anything insider Room, it will fail. \$\endgroup\$ – Austin Hastings Jan 12 '18 at 11:31
  • 1
    \$\begingroup\$ If you delete home_ you'll have a huge bug! This is what ownership is about. The pointer home_ is stored inside an Object, but the object doesn't own the room. So you might say home_ = NULL if you want to clear it out, but only the owner of the room would ever delete the room. \$\endgroup\$ – Austin Hastings Jan 12 '18 at 12:42
  • 1
    \$\begingroup\$ Let's say there's a static vector<Room*> all_rooms inside class Room. Then class Room owns the rooms. The only code that is allowed to delete a room is code in a Room:: method. Objects "point to" rooms, but don't own them. Monsters and users "point to" their current location, but don't own the rooms. Only the Room class owns the rooms, so that's the only place you can delete a room pointer. Simple rule. \$\endgroup\$ – Austin Hastings Jan 12 '18 at 12:45
1
\$\begingroup\$

Use meaningful variable names

To write good code you have to try for your code to speak by it self. You explained that each object needs to keep track of their "homeroom". It's a good idea to keep this information as a private variable within the object as you suggested! But why call it "r"? name it "home_room" or "initial_room"

Be consistent

You use "Room *" in your constructor, but then "Room*" for your different functions. It's a matter of preference but stick to your choice. I personally like "Room* " as it emphasizes on the type.

Use the more modern constructor syntax

You can just do the following to initialize the member varibles

Object::Object(string name, string description, Room* home_room) :  
               name(name), description(description), home_room(home_room) {};

Avoid functions and classes forward declaration, use include when possible

Why do you declare all those functions in main? I think it would be more readable if you wrap all that functionality into a header file and an implementation file.

Avoid variable forward declaration

Avoid the following:

string strExit, strTo, eDigit, rDigit, tDigit; Direction exit, to; bool flag; int cellNum, dirExit, roomNum, index;

It's difficult to read and maintain. Try to declare the variables when they are first initialized.

Fix your code

Whenever you use a new statement, you must include a delete statement, otherwise you have a memory leak as the resources that you are creating with new do not get destroyed (the OS will finally free the memory of your program when it finishes, but your program will be leaking memory during execution)

Use smart pointers

Since C++11 we have access to smart pointers, which ease the problem of having to manually handle all those new and delete.

Rethink whether we really need pointers/smart pointers

You are using a vector to handle the different rooms. This can already dynamically add and remove new rooms (without caring about new and delete). Vector will handle the dynamic allocation for you. So why should we use a pointer in this case?

Avoid "using namespace std"

The whole point of a namespace is to avoid name clashing. Adding "std::" is just a minor effort that makes your code clearer (it explicitly states what you have implemented and what comes from std) an allows you to reuse any name that could be used in std.

Prioritize encapsulation

All the rooms are related using roomConnections. Why not combine this into a class "Floor" or "Storey" that groups and handles the way all these rooms are connected? This way, if you decide to change it, you would only have to work in this room collection class, and main function would still work fine with the its interface. This will also simplify your functions that need to know this

void add(Room &current, AllRooms &allRooms, bool mode)
\$\endgroup\$
  • 1
    \$\begingroup\$ Thank you for the lovely feedback. I'll definitely keep your words in mind moving forward. I tried replacing your code above with my original in Object.cpp and adjust the rest of my code to fit, but for some reason my code still won't compile. \$\endgroup\$ – probeginner Jan 12 '18 at 8:10
  • \$\begingroup\$ Sorry, I didn't fix your code, only gave you tips about how to improve it. For help with fixing bugs you should post in Stackoverflow I think. \$\endgroup\$ – WooWapDaBug Jan 12 '18 at 8:13
  • 1
    \$\begingroup\$ @probeginner According to the Help Center: Code Review is for open-ended questions about code that already works correctly (to the best of your knowledge). Questions seeking help about debugging or understanding code are off-topic for Code Review, and may be on-topic for Stack Overflow \$\endgroup\$ – WooWapDaBug Jan 12 '18 at 8:16
  • \$\begingroup\$ @probeginner Oh, I missed that you already notice this: "Why must the room class have a vector of pointer of Objects? Would not having a vector of Objects suffice? " You are very right about this question. \$\endgroup\$ – WooWapDaBug Jan 12 '18 at 8:16
  • 1
    \$\begingroup\$ Thank you for telling me this; I keep getting stack exchange and stack overflow confused. \$\endgroup\$ – probeginner Jan 12 '18 at 8:18

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