-
linked list problem
i'm trying to create a linked list, the problem is that the return is always returning the id of the newest entry, it should always be returning the same id, because the id of the head should not change. does anyone know what i did wrong?
Code:
int player_list::add_player(player *add)
{
player_entry *now = get_head();
player_entry *p = new player_entry;
p->set_player(add);
if (now == NULL)
{
set_head(p);
}
else
{
while (now->get_next() != NULL)
{
now = now->get_next();
}
now->set_next(p);
return (get_head()->get_player()->get_id());
}
return (1);
}
-
Perhaps it's all in your overly complex return expression.
And the return 1; when you insert the first element in a list is inconsistent.
Most of my list insert functions either return void, or the pointer to the head of the list (depending on how I write them).
-
i know the return is illogical, but thats only there so i could check to make sure it was changing the head, and it is. i don't see where the head is being changed, but its being changed. does anyone know what could be changing it?
-
This function looks sound to me. Perhaps the error is elsewhere.
-
Code:
int player_list::add_player(player *add)
From what I know about coding, shouldnt this be 2 differnt things not 1? I mean like it should be a int then say what the int will = to,but not at ones.
-
i shortened the funciton to this and made a simple program so i can output. this is what i'm finding out.
PHP Code:
int player_list::add_player(player *add)
{
player_entry *now = get_head();
player_entry *p = new player_entry;
if (now == NULL)
{
p->set_player(add);
set_head(p);
cout << "head set to " << get_head() << endl;
}
cout << "head is " << get_head() << endl;
cout << "player is " << get_head()->get_player() << endl;
return (get_head()->get_player()->get_id());
}
here is what i have in main
PHP Code:
int main()
{
player_list players;
if (players.get_head() != NULL)
{
cout << "player is " << players.get_head()->get_player() << endl;
}
else
{
cout << "empty list" << endl;
}
cout << "creating bob . . ." << endl;
int bob = playerCreate(&players, "bob", 4);
cout << "head is " << bob << endl << endl;
if (players.get_head() != NULL)
{
cout << "player is " << players.get_head()->get_player() << endl;
}
else
{
cout << "empty list" << endl;
}
cout << "creating joe . . ." << endl;
int joe = playerCreate(&players, "joe", 4);
cout << "head is " << joe << endl << endl;
cin >> bob;
return (0);
}
here is the function playerCreate
PHP Code:
int playerCreate(player_list *players, string name = "", int si = 0)
{
player temp(name, si);
return (players->add_player(&temp));
}
here is the output
PHP Code:
empty list
creating bob . . .
player set head set
head set to 0x32470
head is 0x32470
player is 0x23ddd0
head is 1
player is 0x23fdd0
creating joe . . .
head is 0x32470
player is 0x23fdd0
head is 2
its saying the the head and the player are the same location in memory both times, which they should be, but it is saying that the head has different values which it shouldn't.
i tried to code it so that the function wouldn't work the second time. if you look closely the second time the player for player_entry is never set, so it doesn't point to anything. knowing that its changing the values stored at the location i figured it was changing the instance pointed to. the new player_entry was never given a player to point to so it shouldn't be able to return an id for it. but somehow it is.
-
> player_list players;
Does your constructor for this set appropriate elements to NULL?
You need to post a whole program, not just the function which crashes.
-
here is the full program:
PHP Code:
#include <string>
#include <iostream>
using namespace std;
enum STATS{BAR2, NATURAL_LIFE, NATURAL_BAR2, STRENGTH, DEXTERITY, STAMINA, VITALITY, INTELECT, SPIRIT, BAR2_TYPE, WEAP_SPEED, WEAP_RANGE, WEAP_POWER, WEAP_ARMOR, RES_FIRE, RES_NATURE, RES_WATER, RES_LIGHT, RES_SHADOW, RES_ARCANE};
enum STAT_ITERATORS{STAT_START = 2, STAT_END = 10, WEAP_START = 11, WEAP_END = 14, RES_START = 15, RES_END = 20};
enum GENERAL{EQUIP_MAX = 1, INVENTORY_MAX = 15};
class player
{
public:
player(string name = "", int si = 0):_name(name){_spriteIndex = si; instanceCount++; id++;}
~player(){instanceCount--;}
player(const player &rhs);
string get_name() const {return (_name);}
void set_name(string name){_name = name;}
int get_spriteIndex() const {return (_spriteIndex);}
void set_spriteIndex(int si){_spriteIndex = si;}
int get_x() const {return (_x);}
void set_x(int x){_x = x;}
int get_y() const {return (_y);}
void set_y(int y){_y = y;}
int get_stat(int index) const {return (_stat[index]);}
void set_stat(int index, int amount){_stat[index] = amount;}
int get_inventory(int index) const {return (_inventory[index]);}
void set_inventory(int index, int itemIndex){_inventory[index] = itemIndex;}
int get_equip(int index) const {return (_equip[index]);}
void set_equip(int index, int itemIndex){_equip[index] = itemIndex;}
static int get_instanceCount() {return (instanceCount);}
static int get_id() {return (id);}
private:
string _name;
int _spriteIndex;
int _x;
int _y;
int _stat[RES_END];
int _inventory[INVENTORY_MAX];
int _equip[EQUIP_MAX];
static int instanceCount;
static int id;
};
player::player(const player &rhs)
{
int i = 0;
_name = rhs.get_name();
_spriteIndex = rhs.get_spriteIndex();
_x = rhs.get_x();
_y = rhs.get_y();
while (i < RES_END)
{
_stat[i] = rhs.get_stat(i);
i++;
}
i = 0;
while (i < INVENTORY_MAX)
{
_inventory[i] = rhs.get_inventory(i);
i++;
}
i = 0;
while (i < EQUIP_MAX)
{
_equip[i] = rhs.get_equip(i);
i++;
}
id = rhs.get_id();
instanceCount++;
cout << "player copied" << endl;
}
int player::instanceCount = 0;
int player::id = 0;
class player_entry
{
public:
player_entry(){_next = NULL;}
player_entry(const player_entry &rhs){cout << "shouldn't be copying";}
player* get_player() const {return (_p);}
void set_player(player *p){_p = p; cout << "player set";}
player_entry* get_next() const {return (_next);}
void set_next(player_entry *next){_next = next; "next set";}
private:
player *_p;
player_entry *_next;
};
class player_list
{
public:
player_list(){_head = NULL;}
player_list(const player_list &rhs){cout << "list copied";}
player_entry* get_head() const {return (_head);}
void set_head(player_entry *h){_head = h; cout << "head set" << endl;}
int drop_player(int id);
int add_player(player *add);
private:
player_entry *_head;
};
int player_list::add_player(player *add)
{
player_entry *now = get_head();
player_entry *p = new player_entry;
if (now == NULL)
{
p->set_player(add);
set_head(p);
cout << "head set to " << get_head() << endl;
}
cout << "head is " << get_head() << endl;
cout << "player is " << get_head()->get_player() << endl;
return (get_head()->get_player()->get_id());
}
int player_list::drop_player(int id)
{
player_entry *now = get_head()->get_next();
player_entry *temp = now->get_next();
player_entry *prev = get_head();
if (prev == NULL)
{
return (0);
}
do
{
if (now->get_player()->get_id() == id)
{
delete now;
prev->set_next(temp);
return (1);
}
else
{
prev = now;
now = temp;
temp = temp->get_next();
}
} while (temp != NULL);
return (2);
}
int playerCreate(player_list *players, string name = "", int si = 0)
{
player temp(name, si);
return (players->add_player(&temp));
}
int playerDrop(player_list *players, int id)
{
return (players->drop_player(id));
}
int main()
{
player_list players;
if (players.get_head() != NULL)
{
cout << "player is " << players.get_head()->get_player() << endl;
}
else
{
cout << "empty list" << endl;
}
cout << "creating bob . . ." << endl;
int bob = playerCreate(&players, "bob", 4);
cout << "head is " << bob << endl << endl;
if (players.get_head() != NULL)
{
cout << "player is " << players.get_head()->get_player() << endl;
}
else
{
cout << "empty list" << endl;
}
cout << "creating joe . . ." << endl;
int joe = playerCreate(&players, "joe", 4);
cout << "head is " << joe << endl << endl;
cin >> bob;
return (0);
}
i've set the _head to NULL. but i guess something else is the problem.
-
You're storing temporary objects. You need to allocate the player objects on the heap, too, or else they won't survive the end of playerCreate.
Thus you have a list full of invalid pointers. The exact reason for your error (confusing player IDs) is that the temporary player object is always created at the same memory location - this is only happenstance, though. The memory in this place is consequently changed quite often - and the ID always seems to be that of the last player created, because that's the data that was last in the place in memory.
-
i changed the function for the playerCreate to
PHP Code:
int playerCreate(player_list *players, string name = "", int si = 0)
{
player *temp = new player(name, si);
return (players->add_player(temp));
}
and that still isn't alloting new memory for it i guess.
-
-
yep the program is saying that the head is the same location and the player is the same location, except the second time the player has a different id.
-
Write out all the memory addresses then, of all player objects.
-
i had it print out the player_entry the player and the player's id. the player_entry and the player were stored in different memory locations and the player's ids were the same.
-
The add_player function is completely broken. Think of what happens when the head is not null.