Compile errors/Class questions.

Started by
7 comments, last by Zahlman 17 years, 5 months ago
Yo, i am getting some compile errors i have no idea how to fix. I don't know why the fighterChoice thing isnt working >.< Can anyone help out?
#include &lt;iostream&gt;
#include &lt;cstdlib&gt;
#include &lt;ctime&gt;
#include &lt;string&gt;
using namespace std;


  class Player //Defining the player class 
  {
        public:
               string name;
               string invent; 
               int hp;
               int dmg;
               int def;
                     };

class Enemy //Define enemy class
  {
        public:
               string name;
               string loot;
               int hp;
               int dmg;
               int def;
               };
int main()

{
    int fighterChoice;
    
    cout &lt;&lt; "Welcome to Redmage Beat' em up!" &lt;&lt;endl;
    cout &lt;&lt; " 1 - Cain (The Fallen Angel)" &lt;&lt;endl;
    cout &lt;&lt; " 2 - Sheeba (The Sand Witch)" &lt;&lt;endl;
    cout &lt;&lt; "Select your fighter:" &lt;&lt; endl;
    cin &gt;&gt; fighterChoice;
    
    if ( fighterChoice == "1")
    
    {
    Player Cain;
    Cain.name = "Cain (The Fallen Angel)";
    Cain.hp = 10;
    Cain.dmg = 1;
    Cain.def = 2;
    }
    
    else if (fighterChoice == "2")
    
    {
    Player Sheeba;
    Sheeba.name = "Sheeba (The Sand Witch)";
    Sheeba.hp = 8;
    Sheeba.dmg = 2;
    Sheeba.def = 2;
    }
  
  cout &lt;&lt; "You chose " &lt;&lt; player.name &lt;&lt; endl;
  cin &gt;&gt; "Is that right? " &lt;&lt;endl;
  
  
    return 0;
    
}
[Edited by - bam104 on November 25, 2006 7:19:54 PM]
Advertisement
"1" is not 1. Since fighterChoice is an integer, it will never be "1". Oh, next time, use the source tags around code to preserve formatting and shorten posts. And telling what compile errors you get would be helpfull too. :)

A note about your classes: do a search for class constructors. They'll make your (coding) life easier. Basically, a constructor is a function that gets called whenever an instance of that class is created, and you can provide constructors that take arguments, to initialize member values with.

For example:

class Player //Defining the player class{public:	// Constructor, allows initializing of the name and hp, dmg and def	Player(string name, int hitpoints, int damage, int definition)	{		this->name = name;		hp = hitpoints;		dmg = damage;		def = definition;	}		string name;	string invent;	int hp;	int dmg;	int def;};


Classes can be used as 'data buckets', e.g. to store some values in, but that's not where their strength lies. You can give them member functions, hide data in them, etc, so basically you can stuff some complex behaviour inside them while all the rest of the code needs to know is what functions to call on them. For a level class, you could read in the level and set each tile variabele, but it's much better to give a level class a LoadFromFile(string filename) function.

Just to give you an idea of what's possible with classes. :)
Create-ivity - a game development blog Mouseover for more information.
First off, I am not a C++ programmer (just plain ol' C), but I can explain this.

1. General rule: Post the compile errors that you're actually getting, so that we don't have to do the compiler's job of trying to find syntax errors in the code.

2. You want to put your code inside ['source] and ['/source] tags (without the single quote). This gives syntax highlighting, and stops it taking up so much space in addition to preserving formatting. Use ['code] and ['/code] tags to insert small code snippets.



3. When you define a class, you're defining what attributes (aka properties) and methods it has. You then create an instance of this class, which is quite separate from the class itself. It's a little clearer if you think of the class as a type. You change the instance, not the class itself (although this can be muddled somewhat if you have static attributes in the class, as these will be shared throughout all the instances of the class, as far as I know - but this is not any concern for you at the moment).

// Defining the classclass Animal{    public:        string Name;        Animal();        ~Animal();        void eat();    private:        string thoughtCurrent;}//...Animal ZooCreature; // Instantiate an instance of the classZooCreature.Name = "Zebra"; // Set the name property of ZooCreature (not Animal) to Zebra.


The problem here is that you create an instance of the class correctly. However, as you define it within the if you end up with each instance only being in scope (visible and accessible) within each of these if statements - so you can't retrieve the name at the end.

There are a number of ways to achieve this, but I'll go with what I think is the correct method in this case (C++ gurus may well correct me here, so don't take this as gospel). You want to declare an instance of the class before any of the if statements, then alter the attributes of this instance inside the if statements, then return the value at the end. See some example code at the end of this post.



4. Hopefully a simpler concept to convey this time. Multiple if statements such as the ones you have can be replaced by a thing called a switch statement. You can see the usage of this in the code below.




Here's an example of what you're looking for (I think):

#include <iostream>#include <string>// We don't need the rest of the includes yet, as you're not using them/* The 'using' statement is generally a Bad Thing. You want to limit its usage   so far as possible, and keep its scope as limited as possible when you do   use it. I've removed it here, so I'll have to include it later. This is   because you dump the entire of the 'std' namespace into scope anywhere in   this source file. */class Player{    public:        string Name;        string Invent;        int HP;        int Dmg;        int Def;};int main(){    // Declare it here, to scope it to the function not just the loop.    int fighterChoice.    do    {        std::cout << "Welcome to Redmage Beat' em up!" << std::endl;        std::cout << " 1 - Cain (The Fallen Angel)" << std::endl;        std::cout << " 2 - Sheeba (The Sand Witch)" << std::endl;        std::cout << "Select your fighter:" << std::endl;        std::cin >> fighterChoice; // Prompt user for choice    }    while((fighterChoice != 1) && (fighterChoice != 2));    /* This is a do..while loop. It will run through the loop once at the start,       and then test the condition to see if it should loop again. In this case,       it checks that the user has entered a valid choice (it checks if       fighterChoice != 1 AND fighterChoice != 2 are true - if they are, then       it'll loop again. This repeats until the user enters a valid choice. */        Player mainPlayer; // Declare an instance of the Player class.    // Switch statement, each case handles a value of fighterChoice    switch(fighterChoice)    {        case 1: // Used if fighterChoice == 1            // Set the attributes of the class instance            mainPlayer.Name = "Cain (The Fallen Angel)";            mainPlayer.HP = 10;            mainPlayer.Dmg = 1;            mainPlayer.Def = 2;            break; /* We need to 'break' to break out of the switch statement,                      otherwise we 'fall through' to the next case. */        case 2:            mainPlayer.Name = "Sheeba (The Sand Witch)";            mainPlayer.HP = 8;            mainPlayer.Dmg = 2;            mainPlayer.Def = 2;            break;    }    std::cout << "You chose " << mainPlayer.Name << std::endl;    std::cout << "Is that right?" << std::endl;    return 0;}


Note that, once again, I'm going to repeat my disclaimer that I am not primarily a C++ programmer, and know only a little of the basic C++ style and syntax. I hope someone more experienced can help you if this isn't right. :)


EDIT: Ach, too slow to post! ;)
[TheUnbeliever]
Thanks for your replies guys. Posting syntax noted. ;)

I had another go before i saw your post TheUnbeliever. But i think your way is better. I was just about to ask for help with this:

#include <iostream>#include <cstdlib>#include <ctime>#include <string>using namespace std;  class Player //Defining the player class   {              public:               string getName; //getters               string getInvent;               int getHp;               int getDmg;               int getDef;                              string setName; //setters               string setInvent;               int setHp;               int setDmg;               int setDef;                                            private:                string name; //data storage                string invent;                int hp;                int dmg;                int def;                                     };                                     //getter definition        string Player:getName() {return name;}        string Player:getInvent() {return invent;}        int Player:getHp() {return hp;}        int Player:getDmg() {return dmg;}        int Player:getDef() {return def;}        //setter definition        void Player::setName (string name);        {             this-> name = name;             }        void Player::setInvent (string invent);        {             this-> invent = invent;             }        void Player::setHp (int hp);        {             this->hp = hp;             }        void Player::setDmg setDef(int dmg);        {             this ->hp = hp;             }        void Player::setDef setDef(int def);        {             this-> def = def;             }        class Enemy //Define enemy class  {        public:               string name;               string loot;               int hp;               int dmg;               int def;               };int main(){    int fighterChoice;        cout << "Welcome to Redmage Beat' em up!" <<endl;    cout << " 1 - Cain (The Fallen Angel)" <<endl;    cout << " 2 - Sheeba (The Sand Witch)" <<endl;    cout << "Select your fighter:" << endl;    cin >> fighterChoice;        if ( fighterChoice == 1)        {    Player Cain;    Cain.setName("Cain (The Fallen Angel)");    Cain.setInvent("Nothing");    Cain.setHp(10);    Cain.setDmg(1);    Cain.setDef(2);        cout << "You chose " << Cain.getName <<endl;    cout << "That k? " <<endl;    cin >> noStop;        }    else if (fighterChoice == 2)        {     Player Sheeba;    Sheeba.setName("Sheeba (The Sand Witch)");    Sheeba.setInvent("Nothing");    Sheeba.setHp(8);    Sheeba.setDmg(2);    Sheeba.setDef(2);    }    cout << "You chose " << Sheeba.getName  <<endl;  cout << "That k? " <<endl;  cin >> noStop;      return 0;    }


I was having problems with the getter definitions. But i think i will go with the loop and switch idea. :)
The getters and setters are actually class methods, not variables like you have declared them. So you'd have like
void setHP(int hp){    this->hp = hp;}int getHP(){    return hp;}


Edit: sorry TheUnbeliever, your reply was better than mine hehe. But you can return strings as you can any other object. You probably got the string class confused with chars. Since you can't return arrays in c++ you have to just return the pointer, which will work but it can be dangerous (like, if the stuff the pointer pointed to was created in the function and suddenly goes BOOM when it's out of scope).
Ah, you appear to have a couple errors there. I'll demonstrate. What you have is effectively this:

class Player{    public:        string getName;        string setName;}


This defines a class MyClass with the attributes getName and setName, both of type string. What you're normally looking to do in this case is encapsulate the data so as that it's only accessible through certain getters and setters. This is not always the best way, but can be useful to say perform certain sanity checks or update the internal state of the object in some way. Here's an example:

#include <iostream>class Person{    public:        // Publicly, we expose the getter and setter methods for the properties        int getAge();        void setAge(int);    private:        /* Privately (not accessible outside the scope of this class), we store           the age (type int) */        int Age;}int Person::getAge() // The class' getter method to get the age{    // Age is within the scope of this method's class, and so it can access it    return Age;}void Person::setAge(int newAge) // The class' setter method to set the age{    Age = newAge;}int main(){    int enteredAge;        do    {        std::cout << "Please enter your age:" << std::endl;        std::cin >> enteredAge;    }    while(enteredAge < 1);    Person newUser;    newUser.Age = enteredAge; /* This will not compile, as the Age property is                                 private to the class (and is therefore only in                                 scope within it) */    // Instead, we must do this:    newUser.setAge(enteredAge); // Sets the class' Age property    std::cout << "You are " << newUser.getAge << " years old." << std::endl;}


You'll also note that I avoided using getters or setters for a string - this is because you can't just straight return a string IIRC, and I can't remember how to have a function return a string, sorry. But it demonstrates the point, I think.


EDIT: Once again, too slow. Heh.
[TheUnbeliever]
Quote:Original post by TheUnbeliever
You'll also note that I avoided using getters or setters for a string - this is because you can't just straight return a string IIRC


You most certainly can; all you have to do is *use a real string*. #include <string> and use std::string. (See my sig also :) )

Anyway, you're doing lots of very painful things. Please stop.

Getters and setters aren't how things are "supposed to work". Initialize your data with constructors. You know how you can write things like "int i = 5;"? In C++, you can also write that as "int i(5);", i.e. using the constructor syntax. When the variable is of an object type, that is the usual way. However, for your own classes, you need to define 'constructors'; you get to choose what parameters are accepted, and then you declare variables by using one of the provided constructors.

A constructor is a member of the class, but it isn't something you invoke upon an instance (object) - because the "current object", in the context of the constructor, *hasn't been created yet - that's what you're doing*. They have some special syntax of their own: So that you can (again) *initialize* the data members of the object (instead of merely assigning to them), there is special syntax provided: the "initializer list".

Now, having created an object, we normally don't want to just offer this get and set logic for each data member, because it *doesn't accomplish anything* compared to just letting the data members be public - you just give yourself more work. Instead, what you are supposed to do is identify *functionality* of the objects. Maybe a Player can .fight(), or .dance(), for example. I don't know; it's your game. In our case, though, it's reasonable to have just the one accessor for a name (the 'get' - don't put a 'set'! Think: should a Player's name *change* over time? No? Then just get it right the first time, i.e. initialize it in the constructor, and leave it alone).

Also, you have problems with scope. Variables are local to the scope in which they are declared, i.e. the deepest level of {}'s that surrounds them. At the end of the scope, the variable "dies". For objects, a "destructor" will be automatically called, if there is one. (These are useful for automating "clean-up" work, but for a while you won't - shouldn't, anyway - be making classes which need that.) Note that if you dynamically allocated something (i.e. 'Foo* x = new Foo();'), the pointer dies at the end of its scope, but the *pointed-at thing* does not. It needs to be explicitly deallocated, with 'delete'. (Also, for array allocations - with new[] - you must use delete[]. The se are separate forms of deletion and they are NOT interchangeable. Under any circumstances.)

Therefore, when you try to get the user's choice, you create an object inside the if-block, but it dies before you can make use of it.

The easiest thing here is to have a dummy Player at the top of the function, and then copy over it with the selected Player. A more refined approach is to have a separate function for selecting the character: in that function, create both options first (that also gives you the information you need to present the first menu), and return one of those objects (the one that is desired).

#include <iostream>#include <string>// You probably shouldn't use namespace std; before your class declarations.// It will make your life harder when you later move things into separate// source files. It should be ok for the main loop though.class Player {  // One common convention is to decorate data members with a leading 'm_'.  // I don't really like it, because it doesn't really provide any information  // that you can't get from the context (and/or the compiler :) ), but it  // does have the advantage that you can then use the 'undecorated' names for  // member functions.  std::string m_name;  std::string m_invent;  int m_hp;  int m_dmg;  int m_def;    public:  // I am keeping just the one accessor:  std::string name() { return m_name; }  // And here is a constructor, which lets us set all the information at once:  Player(const std::string& name, const std::string& invent, int hp, int dmg, int def) : m_name(name), m_invent(invent), m_hp(hp), m_dmg(dmg), m_def(def) {}  // And that shows an initializer list. :)}; // There is nothing left to do for this class :)// BTW, you had all kinds of syntax errors with your getter and setter// implementations before. The best thing is to keep the code simple; then there// are fewer chances to make such typos :)// We don't use the Enemy class yet, so I took it out for now; it doesn't help// with the example.// Here is our helper function where we will create the player:Player chooseFighter() {  // Set up our possible fighters first - this shows use of the constructors  Player Cain("Cain (The Fallen Angel)", "Nothing", 10, 1, 2);  Player Sheeba("Sheeba (The Sand Witch)", "Nothing", 8, 2, 2);  // Present the menu, so we can get the fighter.  // Here we see use of the accessors, to avoid repeating the text info.  // Later, we will generalize the code completely ;)  cout << " 1 - " << Cain.name() << endl;  cout << " 2 - " << Sheeba.name() << endl;  cout << "Select your fighter:" << endl;  int fighterChoice;  cin >> fighterChoice;      if (fighterChoice == 1) {    cout << "You chose " << Cain.getName <<endl;    cout << "That k? " <<endl;    cin >> noStop;    // For now, we don't have any logic for checking if that's OK, so let's    // just assume it is ;)    return Cain;  } else if (fighterChoice == 2) {    cout << "You chose " << Sheeba.getName  <<endl;    cout << "That k? " <<endl;    cin >> noStop;    return Sheeba;  }  // Oops, that wasn't a valid choice. We need to handle this later ;)  throw "Help me, I haven't been shown how to deal with this yet!"}// Now, in the main function, we invoke the function and use the returned Player// object as the Player for the rest of the game.int main() {  cout << "Welcome to Redmage Beat' em up!" << endl;  Player p = chooseFighter();  // This is also a form of initialization - a copy-initialization to be  // precise: the function returns some Player object (which doesn't have a name  // within main()), and 'p' is initialized in such a way that it is a copy of  // that other Player object. Later, in more complicated cases, you will need  // to write special constructors called "copy constructors" to make this work  // properly; but for now there is no need.}
Quote:Original post by Zahlman
Quote:Original post by TheUnbeliever
You'll also note that I avoided using getters or setters for a string - this is because you can't just straight return a string IIRC


You most certainly can; all you have to do is *use a real string*. #include <string> and use std::string. (See my sig also :) )

Anyway, you're doing lots of very painful things. Please stop.


Ah, thanks. I knew about the 'real string', but I misunderstood something I was told. And oops, sorry, I'll stop.
[TheUnbeliever]
Quote:Original post by TheUnbeliever
Ah, thanks. I knew about the 'real string', but I misunderstood something I was told. And oops, sorry, I'll stop.


The "painful things" comment was directed more at the OP, but almost everyone can benefit from putting more effort into good OO design (where OO is used; and into good design in general). :)

This topic is closed to new replies.

Advertisement