rogue like game initializing error

Question

I'm making a simple map for simple rogue-like game. So I need to initialize map with Objects created for each array cell by receiving data from character array[i][j]. Suggested that such CWall, CDoor classes are defined in other files like CWall.cpp, CWall.h, Underneath are the code to initialize in map.cpp

But is this right way to code? I think this causes a problem of allocating memory.

CObject CMap::insertObject(char character){ 
    if (character = '*') {
        CWall cwall;
        return cwall;
    }

    if (character = 'D') {
        CDoor cdoor;
        return cdoor;
    }

    if (character = 'F') {
        CFood cfood;
        return cfood;
    }

    if (character = 'K') {
        CKey ckey;
        return ckey;
    }

    if (character = 'M') {
        CMMonster cmmonster;
        return cmmonster;
    }

    if (character = 'm') {
        CMonster cmonster;
        return cmonster;
    }

    if (character = '@') {
        CPlayer cplayer;
        return cplayer;
    }

    if (character = 'P') {
        CPrincess cprincess;
        return cprincess;
    }

    if (character = '&') {
        CRock crock;
        return crock;
    }

    if (character = 'S') {
        CShield cshield
        return cshield;
    }

    else {
        CShield cshield;
        return cshield;
    }
}

void CMap::initialize(char arr[][COLS]){
    for (int i = 0; i <= 11; i++){
        for (int j = 0; j <= 38; j++){
            char character = arr[i][j];
            insertObject(character);
        }
    }
}

Answer 1

That's not the right way to do it. You are subject to object slicing, since my guess is that your CObject is an object and not a pointer to some object. You need to either return a pointer of type CObject* in your function, then for each case return a new CMonster or new CPlayer etc. Even better, use a smart pointer instead.

What you are trying to implement is called a factory method pattern, see e.g. How to implement the factory method pattern in C++ correctly for more details.

Answer 2

While others correctly pointed out how to code the idea you want coded, I will focus on another thing. Namely, improper usage of polymorphism here. Inheriting everything from meaningless Object smells like Java, and is not welcome in C++. There is simply nothing in common between Princess and Monster (one is kissed, another one is slayed, and to do what with which is up to one's taste), so when both are inherited from Object, it is extremely hard to program the proper game mechanic. You will have to store the actual object type as well (say, an enumeration), and than cast to this type - because only one of them will have method kiss() on them!

The whole code will be a spaghetti of unsafe casts and will be impossible to maintain or reason about.

Instead, go for stongly typed approach. Always know what type is in front of you!

Answer 3

You should be dealing with the data dynamically. What you're doing now has several problems.

A better approach would be:

CObject* CMap::insertObject(char character){ 
    if (character = '*') {
        return new CWall();
    }
...

This will utilize polymorphism to hide the actual class (such as CWall) behind the generic interface (CObject). As you wrote it, each "new" object, such as cdoor, would actually be passed to a copy constructor for CObject. None of which actually accomplished anything meaningful.

Of course, you need to pair these creations with proper destructions down the road.

Answer 4

Well, for one thing, you aren't actually initializing anything. CWall* cwall = new CWall; would be the proper way to dynamically allocate and initialize a new CWall object (assuming CWall has a default constructor, of course), or any object for that matter.

What you also need to bear in mind is that, for everything you allocate with new, you have to deallocate with delete later. Thus, you will need to put some thought into how you store those allocated objects, so you can ensure your destructor or cleanup function removes all of them when you're done. One design pattern to consider would be the Object Pool, though there are easily several dozen good ways to do it. That's footwork you're going to have to do yourself, though, as only you know enough about your project to select the right design pattern. (That book I linked to is a great resource.)

EDIT: As per your comment, there is one other issue, and that is that you are returning different types of objects. This is a simple issue of inheritance - as long as all of those objects inherit from an abstract base class of CObject (or something similar), you can simply list the return type as CObject*. Then, as long as you're returning an object (or a pointer to an object) that inherits from CObject, you're golden.

EDIT 2: Whenever you use new, you are actually getting a pointer to the dynamically allocated object. That creates some problems of its own, including that new can return a null pointer if the allocation fails. Be sure to check for that! A smart pointer can prevent many of those problems, but your use of smart pointers depends on your design pattern choices.