member functions not working properly

Needing help with my text rpg again. I have two classes that I am using to check and assign which decision a user picks. They have 2 choices (warrior or mage). If they choose mage some text is printed, if they choose warrior some different text is printed. My problem is that it only outputs text if they choose mage. Nothing is printed when they choose warrior.

The first class which assigns the information

Code:

---

class Player {
public:
    string getPlayerState(int);
   
private:
    string PlayerState;
};

string Player::getPlayerState(int x){
    if (x==1) {
        PlayerState = "Warrior";
    }
    if (x==2){
        PlayerState = "Mage";
    }
    if (x != 1 && x != 2){
        cout <<"Error, no PlayerState"<<endl;
    }
    return PlayerState;
}

---

And the class that asks the question.

Code:

---

class Game {
public:
    void StartGame();
private:
          int x; //not important
};

void Game::StartGame() {
          string urname;
          int typeClass; // 1 if they pick warrior, 2 if they pick mage

          cout <<"What is your name?"<<endl;
          cin >>urname;

          cout <<"[1] Warrior"<<endl;
          cout <<"[2] Mage" <<endl;
          cout <<"Please make a choice"<<endl;
          cin >>typeClass;

    Player PlayerInfo; // yes I declared them as friends
    PlayerInfo.setPName(urname); // pass's urname to setPName()
    PlayerInfo.getPlayerState(typeClass);

    PlayerInfo.callPlayerUI();
}

---

And the last bit.

Code:

---

Player::callPlayerUI(){
    if (PlayerState == "Mage") {
    cout <<"Something important" <<endl;
    }
    if (PlayerState == "Warrior") {
    cout <<"Totally different text" <<endl;
    }
}

---

Once again, no text is being printed if they pick warrior. I know what the problem is, I just don't know how to fix it. PlayerState never equals "Warrior" and/or int x in getPlayerState never equals 2....even when I explicitly type in 2.

I think thats all thats needed to help me with this problem...let me know if I am missing something.

doh...misplaced }

only took me 4 hours to figure it out

With a cursory glance, it looks okay to me, though it could be improved. I wrote a quick test by converting Game::StartGame() into main() and then fixing the parts that were left out in the example, and it worked. As such, I suggest that you post the smallest and simplest compilable program that demonstrates the problem.

Incidentally, it might be better to have Warrior and Mage subclasses of a Character abstract base class, then have each Player object keep a (smart) pointer to a Character.

Quote:

---

doh...misplaced }

only took me 4 hours to figure it out

---

Ah well, that's why you should have posted a program that actually demonstrates the problem. I would not have wasted time writing one that shows that the problem does not exist. Still, I suggest that you consider using polymorphism instead of type checking.

You are assuming that classes are supposed to print text. They aren't.
Maybe if you make a "Game" class, that's ok, but the rest should absolutely not print text. If something is wrong, they should throw an exception or return an error or just assert.
Also remember that it's less expensive and easier to keep track of things if they're integer-based and not string-based.

>> Maybe if you make a "Game" class, that's ok, but the rest should absolutely not print text.

Logging errors suddenly a bad idea. Wow.

The OOP idea of messaging is also a bit more sophisticated than just blowing up during runtime "when things are wrong."

Quote:

---

Originally Posted by citizen

Logging errors suddenly a bad idea. Wow."

---

That's an entirely different thing. Plus it might be better with an assert so you know where it goes wrong in this case.

And...

Code:

---

    if (PlayerState == "Mage") {
    cout <<"Something important" <<endl;
    }
    if (PlayerState == "Warrior") {
    cout <<"Totally different text" <<endl;
    }

---

This is hardly logging.

You got the point.

Quote:

---

Originally Posted by Elysia

You are assuming that classes are supposed to print text. They aren't.
Maybe if you make a "Game" class, that's ok, but the rest should absolutely not print text.

---

Woah, woah...let me see if I understand what you said.

Class's should not print text UNLESS I'm using a "game" class? Why shouldn't class's print text? I am just asking cause I have a game class and it is going to contain all of my text. I would hate to get all of this done just to find out I did it all wrong.

The thing is that a class is an object. Just like your car. Your operate it and it responds to you, but it never talks.
You must make sure to show that to the user.
There are special classes, you could say however. Like MFC, which wraps an entire app into a class. Obviously, there I would do all the work and print text, etc, because the class was designed to be the main app.

In short, a class is an object--it's defined to be something. A player class is supposed to represent a player, for example, not something that prints text. If you make a "game" class, it should represent the game itself, and so could print text.

Is that a bit clearer?

Quote:

---

Originally Posted by laserlight

With a cursory glance, it looks okay to me, though it could be improved.....

Incidentally, it might be better to have Warrior and Mage subclasses of a Character abstract base class, then have each Player object keep a (smart) pointer to a Character.

Ah well, that's why you should have posted a program that actually demonstrates the problem. I would not have wasted time writing one that shows that the problem does not exist. Still, I suggest that you consider using polymorphism instead of type checking.

---

Sorry you went through all that trouble over a misplaced bracket. I feel bad now :(.

I am coming back from like a 2 year programming break and I am making this little program to springboard myself back into the programming mindset so I just want to get this thing done before I start cleaning it up. I also plan to polish the code once it's complete to learn things I previously didn't know like polymorphism, inheritance and using the heap to keep track of inventoried items. All criticism is more than appreciated but its not quite time for me to ask for that.

@Elysia
So it's more of a "good practice" than a requirement..got ya.

"Good practice" is a bit too big of an umbrella term for what Elysia's talking about. It's more of a design problem. Bit of a crash course on objects and messaging:

Like Elysia said, an object usually represents a thing. So nouns make good objects. You could think of a black box that has stuff inside as an object: while the things inside may define what the box does and what it represents. The stuff inside is also kept as secret as possible (usually) but that's a bit of a different topic.

In a good OOP design, you have to decide whether or not its' appropriate for the object to communicate with the user. Objects that are black boxes communicate to the world (including other objects) through member functions, or whatever might be appropriate for the object to represent its' state. Messaging is a term that defines the way a black box does that.

For example, you could have chosen to let the Player class represent the user's character, and let the Game superobject run the show. Player would be a simple object that can inform the world if the its' alive, or what events it has been doing (such as CastASpell()) and the Game would manage the events of play. Meaning that the Player would inform the Game of its' state exclusively through "messaging" and the Game class would do something, rather than have the Player class interact with the user.

But contrary to what Elysia told you, there may be an appropriate OOP design where Player objects work with the user. It's just that you chose to encapsulate the game as an object, in turn bothering OOP evangelists because they would have made several other design decisions like the ones I detailed for you.

I will state emphatically though that classes can print text and it is appropriate to do so, contrary to Elysia's statement.

Quote:

---

Originally Posted by Elysia

The thing is that a class is an object. Just like your car. Your operate it and it responds to you, but it never talks.
You must make sure to show that to the user.
There are special classes, you could say however. Like MFC, which wraps an entire app into a class. Obviously, there I would do all the work and print text, etc, because the class was designed to be the main app.

In short, a class is an object--it's defined to be something. A player class is supposed to represent a player, for example, not something that prints text. If you make a "game" class, it should represent the game itself, and so could print text.

Is that a bit clearer?

---

No, in fact it doesn't make sense at all. First of all, a "class" is most definitely not an "object". An object is an instance of a class.

Second, I don't know why you're hung up on a class having output functionality. In fact, I'd recommend that he implement a friend operator<< for just such a purpose.

Quote:

---

Originally Posted by medievalelks

No, in fact it doesn't make sense at all. First of all, a "class" is most definitely not an "object". An object is an instance of a class.

---

If you want to be nitpicky, then fine. The instance is an object. But the classes are blueprints for creating those instances, thus the classes themselves can be considered "objects." Whichever fancies your imagination.

Quote:

---

Second, I don't know why you're hung up on a class having output functionality. In fact, I'd recommend that he implement a friend operator<< for just such a purpose.

---

This is an entirely different thing. The class shouldn't output anything. But it can overload the operator and return stuff so the controller of the object can output it. In fact, this might be a good idea because the object itself knows best how it's built and how its data should be interpreted.

Quote:

---

Originally Posted by Elysia

This is an entirely different thing. The class shouldn't output anything. But it can overload the operator and return stuff so the controller of the object can output it. In fact, this might be a good idea because the object itself knows best how it's built and how its data should be interpreted.

---

That's a design issue that depends on the domain.

Quote:

---

Originally Posted by citizen

In a good OOP design, you have to decide whether or not its' appropriate for the object to communicate with the user. Objects that are black boxes communicate to the world (including other objects) through member functions, or whatever might be appropriate for the object to represent its' state.

---

In good OO design, classes encapsulate behavior. Just providing getters and setters of state is not a lot different than passing structs around.

Quote:

---

Originally Posted by medievalelks

In good OO design, classes encapsulate behavior. Just providing getters and setters of state is not a lot different than passing structs around.

---

That was not the point frankly. While I would agree that getters and setters can break encapsulation it depends on what the object is supposed to do, and even then, messaging is still a fundamental OOP concept.

From wikipedia:

Quote:

---

Message passing

"The process by which an object sends data to another object or asks the other object to invoke a method.” [1] Also known to some programming languages as interfacing. E.g. the object called Breeder may tell the Lassie object to sit by passing a 'sit' message which invokes Lassie's 'sit' method.

---

Quote:

---

Originally Posted by citizen

That was not the point frankly. While I would agree that getters and setters can break encapsulation it depends on what the object is supposed to do, and even then, messaging is still a fundamental OOP concept.

From wikipedia:

---

Yes, "sit" is a behavior of dog. Calling member functions to set the position of the dog's legs, hind quarters, tail, etc. to place it in a sitting position is what I'm arguing against. Inside the dog class, it perhaps has-a Mesh, on which operations like this might make sense.

But then again, the outside should probably not be able to control the dog directly. Either you make the object public, or make a get function or just wrapper functions. The latest would provide more encapsulation.

Quote:

---

Originally Posted by Elysia

But then again, the outside should probably not be able to control the dog directly. Either you make the object public, or make a get function or just wrapper functions. The latest would provide more encapsulation.

---

Well, at some point, *something* is going to use the dog's public interface. I was focussing on the granularity of that interface.