Advertisement

Classes w/ members of other classes...

Started by May 29, 2002 06:58 PM
8 comments, last by zackriggle 22 years, 5 months ago
Hello, my name is Zach, and it has been 10 minutes since I have found a runtime error in my program and . . . ''oy . . . sh!t . . My Problem (essentially): WARNING ~60 (est.) LINES CODE CONTENT!!! ////////// // CODE // . . . How do I get those code box thingys ////////// class ITEM { public: int GetStat(int); // Returns stats based on value entered // #defined constants are entered as // parameters . . . very effective void SetStats(int stat, int newvalue); // Same as above, but sets stats private: int give_hp, give_mp, str_up, equip_where, // To determine where an item can be eqpped ID; // For use in inventory situations // so as not to delete the wrong item with the // same name . . . etc . . . // This prevents: // (DELETE) Staff of Fire (+3) -- 13-400 damage // instead of the intended delete // Staff of FIre (+3) -- 13 - 40 damage . . . }; class PLAYER { public: void EquipItem(ITEM& item_2_b_equipped, int const_defined_value); // The const_defined is a defined for WHERE to equip it // Items can only be equipped if they are in the inventory void GiveItem(ITEM& item); // adds item to inventory // Should be private: but for simplicity, it will be public list inventory; // Both for manipulating inventory list::iterator Icurrent; ITEM head, body, weapon; // Etc, etc }; // Now that we got past that ... void myfunction() { PLAYER player; ITEM item; // Then set the stats for player + item . . . yadayada // Let''s leave that out . .. . player.GiveItem(item); Icurrent = inventory.begin(); // The we search the inventory till we find the item . . . // We''ll leave that out . . . ////////////////////////// // ERROR HERE // //////////////////////////////////////////////////////// player.EquipItem((*Icurrent), (*Icurrent).GetStat(EQUIPPABLE_WHERE)); ///////////////////////////////////////////////////////// // And it should equip the item, but it does not, and ends up // With a default value in the switch() statement // Is this a pointer error or simple error in the switch() // That cannot be found by the compiler . . . (it compiles fine) // HELP ////////////// // END CODE // ////////////// Why don''''t we just kill the guys who write error messages. It''''d make our lives a hell of a lot easier **************************** Gimme a break, I''''m Just 13 (14 in JULY)
Most importantly:
use (source) and (/source) - use [] instead of ()
Read the faq "http://www.gamedev.net/community/forums/faq.asp"

You have a few pointer errors in your code.

You probably your player to contain pointers to all the items s/he carries, not the actual items themselves. This allows you to move the object between inventory, player''s apparel, and the world without having to make copies. It prevents duplicates, and corruption of the object. It is the standard way to do this.

Therefore.
ITEM *head, *body, *weapon; etc...
player.EquipItem(Icurrent, Icurrent->GetStat(EQUIPPABLE_WHERE));
EquipItem(ITEM *item_2_b_equipped, int const_defined_value);

If you use pointers this way, you will avoid bugs like the one you have now.

BTW: You never use & in function parameters.

(I''m not sure about the template stuff, never used em)

You might want to post more of your code, such as the body for EquipItem etc.
Advertisement
Wheres my run time error?

looked to me like you were using begin and dereferencing the iterator without checking to see if there was anything in the inventory.

if list is empty and you use .begin() there is a run time error.

if list was empty and you got an iterator then you dereferenced it there is a run time error.

solution before using .begin(). use if (!.IsEmpty()) {..}


In the real, lengthy code I do check to see if there is anything in the inventory . . . srry bout that
quote: Original post by Krylloan
BTW: You never use & in function parameters.


Pick up your favourite C++ book and read about references. The & operator is perfectly acceptable in method parameters.
not only acceptable but also very useful. But remember - if you are passing by reference to improve performance but aren''t planning on writing to the data make it a const reference so the compiler can catch erroneous attempts to do so.

-

Geocyte Has Committed Suicide.
Geocyte Has Committed Suicide.
Advertisement
Sorry about that comment on &''s. Basically, a tutor told me it was incorrect syntax.

I however think it is hideously ambiguous. It is utterly superfluous to "fn(var *vec) { vec->x = 0; }", and can mislead the user of the function to believe the function does not modify their variable. But that''s irrelevent to the topic so I''ll stop there.

  [source ][/source ]  
quote: Original post by declspec
if list is empty and you use .begin() there is a run time error.

Please, RTFM before you misinform others. If a sequence is empty and .begin() is called on it, it returns an iterator pointing at the same location as .end(). One simply needs to check that the iterator received is never equivalent to that returned by .end(), as follows:
std::list< some_type >::iterator start, stop;start = some_list.begin(), stop = some_list.end();while( start != stop ){  // logic here  ++start;} 

If .begin() returns the same iterator as .end(), the while loop never executes. (But you already knew that, didn''t you...?)
A direct correction of your problem, along with some general pointers.

class PLAYER{public:...   list<ITEM>::iterator Icurrent;...}; 

You shouldn''t be storing this iterator in your class. It''s not a logical part of your class, and as class data it may contain invalid data at times (after any change to the list sequence, it is potentially invalidated).

void myfunction(){...   Icurrent = inventory.begin();   // The we search the inventory till we find the item . . .... 

What happens if you don''t find the item? Do you check for the iterator being equivalent to .end() and exit on that? If you don''t, that''s your error: you''re using an invalid iterator (and dereferencing it).

...(*Icurrent).GetStat(EQUIPPABLE_WHERE));... 

This can be rewritten as Icurrent->GetStat(...); Cleaner syntax, IMO.

quote:
Why don''t we just kill the guys who write error messages. It''d make our lives a hell of a lot easier

Why don''t we just kill the idiots who don''t read the explanations of error codes in the manual, or worse, don''t know the language? It''d make our lives and thiers a hell of a lot easier.

Flippant, it''s-the-compiler''s-fault attitudes irritate me. You don''t know what you''re doing, you''re new to this; whose fault is it likely to be?

This topic is closed to new replies.

Advertisement