   battle game
  heres a battle game i just started programing i just started learning about classes its very messy how could i clean it up a bit#include <iostream>#include <string>using namespace std;class player{    private:    int hp;    int xp;    int str;    string name;    int gp;    public:    int getgp()    {  return gp;    }    void setgp(int x)    {   gp = x;    }    int gethp()    {   return hp;    }    int getxp()    {   return xp;    }    string getname()    {   return  name;    }    int getstr()    {   return str;    }    void setname(string a)    {   name = a;    }    void sethp(int x)    {   hp = x;    }    void setxp(int x)    {   xp = x;    }    void setstr(int x)    {   str = x;    }    void inchp(int x,char b) //hp changer    {   if (b == '-')   {  hp = hp - x;   }   else if (b == '+')   {  hp = hp + x;   }    }    void incgp(int x,char y)    {   if (y == '+')   {  gp = gp + x;   }   else if (y == '-')   {  gp = gp - x;   }    }};int main(){    string cmd;    player a;    a.sethp(10);    a.setxp(0);    a.setstr(10);    a.setgp(100);    cout << "welcome to this game" << endl;    cout << "enter your name" << endl ;    {    string temp;    cin >> temp;    a.setname(temp);    }    bool mainmenu = true;    while (mainmenu)    {    cout << "hp = " << a.gethp() << " gp = " << a.getgp() << endl;    cout << "what do you want to do " << endl;    cout << "type hos to go to hospital" << endl;    cout << "type stats to view stats " << endl;    cout << "type start to start adventure" << endl;    cout << "type exit to exit game " << endl;    cin >> cmd;    if (cmd == "hos")    {   cout << "welcome to the hospital" << endl;   cout << "cost 10 gp to heal 5 hp to you want to " << endl;   cout << "yes or no" << endl;   cin >> cmd;   if (cmd == "yes")   {  if ((a.getgp()) >= 10)  { if (a.gethp() == 100) {cout << "you are fully heald" << endl; } else {a.inchp(5,'+');a.incgp(10,'-'); }  }  else  { cout << "you dont have anouf mony " << endl;  }   }}    else if (cmd == "stats")    {   cout << "gp = " << a.getgp() << endl;   cout << "hp = " << a.gethp() << endl;   cout << "str = " << a.getstr() << endl;   cout << "xp = " << a.getxp() << endl;   cout << "name = " << a.getname() << endl;    }    else if (cmd == "exit")    {   mainmenu = false;    }    else if (cmd == "start")    {    }    else {   cout << "invalid command" << endl;    }    }}[Edited by - burgert on June 10, 2009 6:36:02 PM]
  I'm pretty new to programming myself, but the first thing I noticed when looking through your code was that you don't use very descriptive names for function parameters.  For example, instead of void getname(string a)you could instead use:void getName(string newName)You might also want to look into to using switch statements for handling player responses.;  very nice for a first game, keep it up.Just a few nit picky advice.1: Using the " using namespace ", is generally a bad idea and defeats the purpose of the namespace in the first place.2: Making player a struct instead of a class would make thing alot easier for the way you use it. There's no need to hide the data if it's completely accessible. Also If you write both setters/accessors that are basically the same thing and the = operator, why not make the data public. ie.player.setHP( 10 );hp = player.getHP();//could just beplayer.HP = 10;hp = player.HP;// and would save you having to write a bloated class with 2 functions for every piece of data3: just like the guy above, I prefer using full names for everything so I can easily see what it is. Also the functions getData(), using the name get seems odd every time I read it. ie use if (x >= player.STR()) instead.4: Your hp and gp changer functions, could simply bevoid SetHP(int x){    HP += x;}//If you want to lower the hpSetHP( -10 );;  think you for all advise lol i cant believe i didn't think about passing negative numbers into my get functions   ;Quote:Original post by freeworld1: Using the " using namespace ", is generally a bad idea and defeats the purpose of the namespace in the first place.Nothing wrong with "using namespace" within a .cpp file or a function.As long as it's not declared in a header file, you're safe that it wont leak unto any other code that may have conflicting functions.   ;Quote:Original post by freeworld2: Making player a struct instead of a class would make thing alot easier for the way you use it. There's no need to hide the data if it's completely accessible. Also If you write both setters/accessors that are basically the same thing and the = operator, why not make the data public. ie.Indeed, if you're only writing getters and setters, you might as well make the data public.Or you could write actually object-oriented code, and expose functionality rather than data. Much more useful:class Player{public:   // Use constructors to initialize member variables with sense-making values,   // here I'm using an initializer list, but you can also add code to the   // constructor body itself. Oh, and keep in mind that the compiler will   // generate a default constructor, destructor, copy constructor and assignment operator   // if you don't create them - and these don't always do what you want   // (this becomes especially important once you're working with dynamic memory allocation).   Player() : max_hitpoints(100), hitpoint(100), alive(true)   {   }   // Outside code doesn't get to tinker with the hp directly - that's the Player class'   // responsibility. Note how it's possible to change the damage calculations   // without having to change outside code. For example, if you add an armor variable,   // you can use it's value to lower the damage taken. If hitpoints was a public   // member, then code like that would pop up everywhere in the game... which is   // a nightmare to maintain.   void DoDamage(int damage)   { hitpoints -= damage; if(hitpoints < 0)    alive = false; else if(hitpoints > max_hitpoints)    hitpoints = max_hitpoints;   }   int Hitpoints() { return hitpoints; }private:   int max_hitpoints;   int hitpoints;   bool alive;};EDIT: Just a comment:Quote:void SetHP(int x){    HP += x;}This does not set the hp, but add to the hp. SetHP is a deceptive name for this function. Deceptive names are not good. The same goes for 'void getName(string newName)': getName? You probably mean setName instead. Yes, it's nitpicking, and yes, it's important. You're going to hate subtle mistakes like that if you need to fix bugs in someones (perhaps your own) code a few months later. ;)   ;  what is a struct how is it different then a class i just started learning about classes   ;  A suggestion.  In your setter methods, do some testing.  Is it valid to set xp to -1000?  How about gp or hp or anything else?  How about testing if the string is null?  That is the place you want to make sure your values are valid.Next do you really want them to just be able to set them?  Or modify them?  Should xp start at 0 and then be modified by a given value or set the total?  Same for all the others.theTroll   ;Quote:Original post by burgertwhat is a struct how is it different then a class i just started learning about classesAll the members are public by default.  That is the biggest issue at your level.theTroll;Quote:Original post by burgertwhat is a struct how is it different then a class i just started learning about classesLike said already all the data is automatically public, but just as a class you can set things as private, you can also make cunstructors and other functions in your struct.I could be wrong here but I thought the only major difference was structs cant use inheritance like a class can??? I'm sure someone else can explain better or your good friend google.   