Using list made by classes

Question

Well, I just started learning c++ and i seem to have some problem. To be specific i have to make a program that recreates the game musical chairs. For this i was supposed to make two classes one named member that would have the position of a player and their id number and also point to the next (last member should point to first.). Second a class named chain that would point at the first member and also have the total number of exsting players. For starters i should create the chain based on a parametre N that would give every member a random id and position them and of course link the powers with each other.

What i wrote was

#include <iostream>
#include <stdlib.h>
#include <time.h>

using namespace std;

class Member{

public:
    int position, number;
    Member  *next_member;

    void Member2( Member *member_, int pos, int id,int N){
        if(pos <= N){
            member_->position=pos;
            member_->number=id;
            Member2 (member_->next_member, pos++, rand(), N);
            if(pos == N)
                member_->next_member = this;
        }
    }
};

class Chain {
    Member *First_member;
    int size;
public:

    Chain ( int N){
        size = N;
        srand(time(NULL));
        First_member->Member2(First_member, 1 , rand(), N);
    }
};

and the main just called chain.

The problem is that when Member2 is called by itself, the whole thing crashes. Any help is good.

Edit: When trying to debug it, it seems there is segmentantion fault when membber_ is used after Member2 has called the Member2 isnide it.

your pointers don't point to anything. You need to use dynamic memory allocation or something like unique_ptr which handles that for you. — user1937198, Mar 01 '14 at 21:30
Also see http://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice on why you shouldn't have `using namespace std;` at the top of your code. — user1937198, Mar 01 '14 at 21:32
By called by itself i am reffering to the part that Member2 is calling Member2. — George 0X, Mar 01 '14 at 21:32
still not really sure how i should do dynamic memory allocation... an example would be usefull. — George 0X, Mar 01 '14 at 21:39

score 0 · Answer 1 · answered Mar 01 '14 at 21:34

0

The expression pos++ uses post increment, which produces the original value of pos as the expression result. Thus the pos argument does not increase in the recursive call. Thus, you get infinite recursion, which if you're lucky crashes when it's used up all stack space.

answered Mar 01 '14 at 21:34

Cheers and hth. - Alf

142,714
15
209
331

though when I had it print the member_->position and member_->number after the member_->positin=pos; member_->number=id; part it only printed them when member2 was called from chain and it crashed before printing anything.... if it was in an infinite recursion shouldn't it also print things infinitely before crashing? – George 0X Mar 01 '14 at 21:38
@George0X: The code you have shown has infinite recursion, as described in this answer. I do not know about the code you're talking about, that had some output statements. Since the code has many issues, including UB due to use of indeterminate pointer values, it might crash even when the infinite recursion issue is fixed. – Cheers and hth. - Alf Mar 01 '14 at 21:50

score 0 · Answer 2 · answered Mar 01 '14 at 21:40

There are several issues (at least) with that code:

1)

Member *First_member;

is only a declaration. To turn it into a definition, you need to actually allocate memory, e.g:

Member *First_member = new Member;

and also release it in the end, e.g. (not necessarily the best way to do it, but my C++ is a little rusty):

void free(Member* _member) {
    Member* _next = _member->next_member;
    if (_next!=NULL) free(_next);
    delete _member;
}

Member* First_member = new Member;
// some code ...
free(First_member);

2) More serious problem is your Member2 method: it does not check whether its Member *member_ arguments is not NULL (in which case the code will indeed crash, because it tries to work with garbage as if it was data).

3) Member2 should also take care about allocating memory for the next element in the chain, e.g.:

if (member_->next_member == NULL) {
    member->next_member = new Member;
}
Member2 (member_->next_member, pos++, rand(), N);

Using list made by classes

2 Answers2