Sign in to follow this  
burgert

battle game

Recommended Posts

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]

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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 be

player.HP = 10;
hp = player.HP;

// and would save you having to write a bloated class with 2 functions for every piece of data




3: 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 be


void SetHP(int x)
{
HP += x;
}

//If you want to lower the hp
SetHP( -10 );


Share this post


Link to post
Share on other sites
think you for all advise lol i cant believe i didn't think about passing negative numbers into my get functions

Share this post


Link to post
Share on other sites
Quote:
Original post by freeworld
1: 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.

Share this post


Link to post
Share on other sites
Quote:
Original post by freeworld
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.

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. ;)

Share this post


Link to post
Share on other sites
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

Share this post


Link to post
Share on other sites
Quote:
Original post by burgert
what is a struct how is it different then a class i just started learning about classes


All the members are public by default. That is the biggest issue at your level.

theTroll

Share this post


Link to post
Share on other sites
Quote:
Original post by burgert
what is a struct how is it different then a class i just started learning about classes


Like 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.

Share this post


Link to post
Share on other sites
Quote:
Original post by freeworld
Like 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.

Correct. In C++, classes and structs are nearly identical.

Quote:
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.

They can. They just default to public inheritance, unlike classes (but then again, you can specify what kind of inheritance to use).

Personally, I prefer to use structs for PODs (Plain Old Data structure) - to indicate that it's just a bucket of data that I'm sending around. For anything else, I use classes - and, as demonstrated above, I think about how they will be used by other code, not in the first place about the exact data that they will contain. Basically, I try to hide as much data as possible. Other code doesn't need to see how they look from the inside, it just needs to be able to use those classes.

I wouldn't put a chocolate bar right into your stomach, right? I'd give it to you - and you know what to do with it. I don't need to worry about that. ;)

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this