Problem with a class

Started by
7 comments, last by Zahlman 18 years, 3 months ago
Hiya I have read the text rpg tutorial and am currently trying to make my own along the same lines as that. I have made the Player, and now the Armour and Weapons, but after making these I seem to get many errors thta I didn't before. It seems that the compiler takes all global variables and functions to be parts of the Armour class. Can you please tell me whats wrong. Armour.h


#ifndef ARMOUR_H
#define ARMOUR_H
#include "libraries.h"

class Armour : public Item
{
private:
    int defense;
public:
    Armour()
    {
    defense = 0;
    );
    void SetDefense(int amount)
    {
    defense = amount;
    };
    int GetDefense()
    {
    return defense;
    };
    CreateArmour(char* newname, int newdefense, int newprice)
    {
        SetName(newname);
        SetDefense(newdefense);
        SetPrice(newprice);
    };
};

#endif


Inn.cpp (where the errors are)


#include "libraries.h"

bool OverHear = false;
Player MyPlayer;

int Inn()
{
	if (OverHear == false)
	{
		std::cout << "Lots of Text";
		OverHear = true;
		MakePlayer(&MyPlayer);
	}
	
	
	std::cout << "More Text";
	std::cout << "\n\n> ";
	
	int choice = 0;
	getchar();
	std::getchar();
	return 0;
}


These are the errors (I'm using bloodshed dev C++): 3 C:\DOCUME~1\Owner\MYDOCU~1\PROGRA~1\C__~1\TOBEAL~1\Inn.cpp ISO C++ forbids initialization of member `OverHear' 3 C:\DOCUME~1\Owner\MYDOCU~1\PROGRA~1\C__~1\TOBEAL~1\Inn.cpp making `OverHear' static 3 C:\DOCUME~1\Owner\MYDOCU~1\PROGRA~1\C__~1\TOBEAL~1\Inn.cpp ISO C++ forbids in-class initialization of non-const static member ` 7 C:\DOCUME~1\Owner\MYDOCU~1\PROGRA~1\C__~1\TOBEAL~1\Inn.cpp `int Armour::Inn()' and `int Armour::Inn()' cannot be overloaded 24 C:\DOCUME~1\Owner\MYDOCU~1\PROGRA~1\C__~1\TOBEAL~1\Inn.cpp parse error at end of input C:\DOCUME~1\Owner\MYDOCU~1\PROGRA~1\C__~1\TOBEAL~1\Makefile.win [Build Error] [Inn.o] Error 1
Whether you think you can or think you can’t, you’re probably right – Henry Ford
Advertisement
Looks like a problem in libraries.h (or another file included from within libraries.h), probably an unclosed class definition. Check all your class definitions. If that doesn't help, try posting the contents of libraries.h.

Enigma
I won't recommend having global variables in your program, but to make it work, you should declare it as static variable. This ensures there's only one copy of the variable throughout it's life:
static bool OverHear = false;
To make use of OverHear in other source files, you include an extern statement:
extern bool OverHear ;


[Edit]: On a second look, Enigma is probably right. Did you check the libraries.h ?.

[Edited by - Skeleton_V@T on January 11, 2006 11:15:13 AM]
--> The great thing about Object Oriented code is that it can make small, simple problems look like large, complex ones <--
Skeleton_V@T: Not if it's not a class member. namespace scope static means one copy per translation unit (a translation unit is basically but not exactly a .cpp file and related includes) and is deprecated in favour of anonymous namespaces (namespace {type variable = initialiser;}).

Enigma
The contents of Libraries.h are (very closely based upon Wrathlands from the text RPG tutorial):

#ifndef LIBRARIES_H#define LIBRARIES_H#include <iostream>using namespace std;#include "Item.h"#include "Weapon.h"#include "Armour.h"#include "Entity.h"#include "Player.h"#include "Globals.h"#endif


Item.h
#ifndef ITEM_H#define ITEM_Hclass Item{private:    char name[32];    int cost;public:    Item(){cost = 0;};    char* GetName(){return name;};    void SetName(char* newname){strcpy(name,newname);};    int GetPrice(){return cost;};    void SetPrice(int price){cost = price;};};#endif

Weapon.h
class Weapon : public Item{private:    int damage;public:    Weapon()    {    damage = 0;    };    int GetDamage()    {    return damage;    };    void SetDamage(int amount)    {    damage = amount;    };    void CreateWeapon(char* newname, int newdamage, int newprice)    {        SetName(newname);        SetDamage(newdamage);        SetPrice(newprice);    };};

Entity.h
#ifndef ENTITY#define ENTITY#include "Libraries.h"class Entity{private:    char name[32];    int gold;    int exp;    int health;    int maxhealth;    int mana;    int maxmana;    int strength;    int reactions;    int will;    bool poisoned;public:    bool SetName(char* newname);    char* GetName();        void SetGold(int amount);    void SpendGold(int amount);    void AddGold(int amount);    int GetGold();        void SetExp(int amount);    void AddExp(int amount);    int GetExp();        void SetHealth(int amount);    void AddHealth(int amount);    void LooseHealth(int amount);    int GetHealth();        void SetMaxHealth(int amount);    int GetMaxHealth();        void SetMana(int amount);    void AddMana(int amount);    void LooseMana(int amount);    int GetMana();        void SetMaxMana(int amount);    int GetMaxMana();        void SetStrength(int amount);    int GetStrength();        void SetWill(int amount);    int GetWill();        void SetReactions(int amount);    int GetReactions();        void SetPoisoned(bool value);    bool IsPoisoned();};#endif

Player.h
class Player : public Entity{private:    Weapon *weapon;    Armour *armour;//    POTION *potionlist[15];//    SPELL *spelllist[10];    int level;public:    Player();    Weapon* GetWeapon();    void SetWeapon(Weapon newweapon);    Armour* GetArmour();    void SetArmour(Armour newarmour);    //Weapon spell and potion stuff here};

Any suggestions?
Whether you think you can or think you can’t, you’re probably right – Henry Ford
Quote:Original post by Peter Conn
Any suggestions?


0) Use std::string to represent names and other such bits of text.

1) Get it working with a much smaller set of "things" first. And as mentioned, check that all your {}'s balance. (Also, you don't need semicolons after function declarations - only after classes.)

2) Don't write all those get and set methods. That's not how OO is supposed to work. (If you want a PSP for your birthday, you don't get your parents' wallet, remove the money and head down to the store, because (a) that's immoral and (b) it will cause huge problems for your parents when they later expect to be able to buy something else. Instead, you ask your parents to buy you the PSP. Proper OO workings are quite analogous.)
Quote:Original post by Zahlman
2) Don't write all those get and set methods. That's not how OO is supposed to work. (If you want a PSP for your birthday, you don't get your parents' wallet, remove the money and head down to the store, because (a) that's immoral and (b) it will cause huge problems for your parents when they later expect to be able to buy something else. Instead, you ask your parents to buy you the PSP. Proper OO workings are quite analogous.)


I was thinking about all of that, I just did that because its supposedly what should be done, but I don't see any danger of, if I'm the only person working on a project just keeping all class variables public, as it would save about an hours worth of typing.
Whether you think you can or think you can’t, you’re probably right – Henry Ford
Also I have changed all that stuff, the code does seem easier to understand now, but the compiler still treats Inn.cpp as part of the Armour class.

Armour.h
#ifndef ARMOUR_H#define ARMOUR_H#include "libraries.h"class Armour : public Item{public:    int defense;    Armour()    {    defense = 0;    );    CreateArmour(std::string newname, int newdefense, int newprice)    {        name = newname;        defense = newdefense;        cost = newprice;    };};#endif


Inn.cpp

#include "libraries.h"bool OverHear = false;Player MyPlayer;int Inn(){	if (OverHear == false)	{		std::cout << "Lots of Text";		OverHear = true;		MakePlayer(&MyPlayer);	}			std::cout << "More Text";	std::cout << "\n\n> ";		int choice = 0;	getchar();	std::getchar();	return 0;}


And the errors are still:

3 C:\Documents and Settings\Owner\My Documents\Programming\C++\To Be A Legend\Inn.cpp
ISO C++ forbids initialization of member `OverHear'
3 C:\Documents and Settings\Owner\My Documents\Programming\C++\To Be A Legend\Inn.cpp
making `OverHear' static
3 C:\Documents and Settings\Owner\My Documents\Programming\C++\To Be A Legend\Inn.cpp
ISO C++ forbids in-class initialization of non-const static member `
7 C:\Documents and Settings\Owner\My Documents\Programming\C++\To Be A Legend\Inn.cpp
`int Armour::Inn()' and `int Armour::Inn()' cannot be overloaded
24 C:\Documents and Settings\Owner\My Documents\Programming\C++\To Be A Legend\Inn.cpp
parse error at end of input
C:\Documents and Settings\Owner\My Documents\Programming\C++\To Be A Legend\Makefile.win
[Build Error] [Inn.o] Error 1
Whether you think you can or think you can’t, you’re probably right – Henry Ford
Quote:Original post by Peter Conn
Quote:Original post by Zahlman
2) Don't write all those get and set methods. That's not how OO is supposed to work. (If you want a PSP for your birthday, you don't get your parents' wallet, remove the money and head down to the store, because (a) that's immoral and (b) it will cause huge problems for your parents when they later expect to be able to buy something else. Instead, you ask your parents to buy you the PSP. Proper OO workings are quite analogous.)


I was thinking about all of that, I just did that because its supposedly what should be done, but I don't see any danger of, if I'm the only person working on a project just keeping all class variables public, as it would save about an hours worth of typing.


Well. The whole point is that those get and set functions are likely to be "an hour's worth of typing" *without real benefit*. But designing functions with proper, well, functionality, is unbelievably valuable.

If you think you don't need to protect yourself from yourself, well, I'll let you learn the hard way. ;)

(Although to be fair, when you don't know what functionality is needed, often the best way is to just write stuff, look for patterns in the stuff your wrote, and then put together member functions to do it.)

This topic is closed to new replies.

Advertisement