Thread: linked list problem

  1. #1
    Registered User
    Join Date
    Nov 2005
    Posts
    137

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

  2. #2
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    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).

  3. #3
    Registered User
    Join Date
    Nov 2005
    Posts
    137
    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?

  4. #4
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,895
    This function looks sound to me. Perhaps the error is elsewhere.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  5. #5
    Registered User
    Join Date
    Dec 2005
    Posts
    155
    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.

  6. #6
    Registered User
    Join Date
    Nov 2005
    Posts
    137
    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 *= 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 *playersstring name ""int si 0)
    {
        
    player temp(namesi);
        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.
    Last edited by yahn; 12-24-2005 at 02:14 PM.

  7. #7
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    > 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.

  8. #8
    Registered User
    Join Date
    Nov 2005
    Posts
    137
    here is the full program:

    PHP Code:
    #include <string>
    #include <iostream>

    using namespace std;

    enum STATS{BAR2NATURAL_LIFENATURAL_BAR2STRENGTHDEXTERITYSTAMINAVITALITYINTELECTSPIRITBAR2_TYPEWEAP_SPEEDWEAP_RANGEWEAP_POWERWEAP_ARMORRES_FIRERES_NATURERES_WATERRES_LIGHTRES_SHADOWRES_ARCANE};
    enum STAT_ITERATORS{STAT_START 2STAT_END 10WEAP_START 11WEAP_END 14RES_START 15RES_END 20};
    enum GENERAL{EQUIP_MAX 1INVENTORY_MAX 15};

    class 
    player
    {
      public:
        
    player(string name ""int si 0):_name(name){_spriteIndex siinstanceCount++; 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 indexint amount){_stat[index] = amount;}
        
        
    int get_inventory(int index) const {return (_inventory[index]);}
        
    void set_inventory(int indexint itemIndex){_inventory[index] = itemIndex;}
        
        
    int get_equip(int index) const {return (_equip[index]);}
        
    void set_equip(int indexint 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 (
    RES_END
        {
            
    _stat[i] = rhs.get_stat(i);
            
    i++;
        }
        
        
    0;
        while (
    INVENTORY_MAX)
        {
            
    _inventory[i] = rhs.get_inventory(i);
            
    i++;
        }
        
        
    0;
        while (
    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";}
        
    playerget_player() const {return (_p);}
        
    void set_player(player *p){_p pcout << "player set";}
        
        
    player_entryget_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_entryget_head() const {return (_head);}
        
    void set_head(player_entry *h){_head hcout << "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 *= 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 *playersstring name ""int si 0)
    {
        
    player temp(namesi);
        return (
    players->add_player(&temp));
    }

    int playerDrop(player_list *playersint 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.

  9. #9
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,895
    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.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  10. #10
    Registered User
    Join Date
    Nov 2005
    Posts
    137
    i changed the function for the playerCreate to

    PHP Code:
    int playerCreate(player_list *playersstring name ""int si 0)
    {
        
    player *temp = new player(namesi);
        return (
    players->add_player(temp));

    and that still isn't alloting new memory for it i guess.

  11. #11
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,895
    You sure?
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  12. #12
    Registered User
    Join Date
    Nov 2005
    Posts
    137
    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.

  13. #13
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,895
    Write out all the memory addresses then, of all player objects.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  14. #14
    Registered User
    Join Date
    Nov 2005
    Posts
    137
    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.

  15. #15
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,895
    The add_player function is completely broken. Think of what happens when the head is not null.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Need help sorting a linked list. Beginner
    By scarlet00014 in forum C Programming
    Replies: 1
    Last Post: 09-27-2008, 06:16 PM
  2. singly linked circular list
    By DarkDot in forum C++ Programming
    Replies: 0
    Last Post: 04-24-2007, 08:55 PM
  3. Replies: 6
    Last Post: 03-02-2005, 02:45 AM
  4. Linked list with two class types within template.
    By SilasP in forum C++ Programming
    Replies: 3
    Last Post: 02-09-2002, 06:13 AM
  5. singly linked list
    By clarinetster in forum C Programming
    Replies: 2
    Last Post: 08-26-2001, 10:21 PM