Adding nodes inside a linked list using a function

Question

I try to use the CreateRoom function to adding new nodes.
Each time I add a node, I edit the old "lastRoom.next" and make it the current new node's address.

And then I make the current new node's pointer as the new "lastRoom"
I thought this is a good idea and I don't need to return anything. I thought this is good.
However, it doesn't work at all.

I'm really bad at coding and I just learn C. Can anyone help?

struct Room {
    int number;
    int status;
    int luxury;
    char occupier[20];
    struct Room *next;
};
//I main copy this kind of code from my lecture notes.
//But It's really crappy. This is the only example I can get from the lecture notes
typedef struct Room Room_t;
Room_t *newRoomNode, *ptr, *prev, *temp;
Room_t *firstRoom = NULL, *lastRoom = NULL;

Room_t* CreateRoom(Room_t **first, Room_t **last, int RoomNumber, int type){
    Room_t *new = (Room_t *)malloc(sizeof(Room_t));
    int *temp;
    if (new == NULL)
    {
        printf("\nMemory was not allocated");
        return 0;
    }
    else
    {
//And then I try my way of adding new nodes. 
//I don't really understand the example so I make my own
        if (*last != NULL)
        {
            (**last).next = new;
        }
        new->number = RoomNumber;
        new->luxury = type;
        new->next = NULL;
        *last = new;
        if (*first=NULL){
            *first=new;
        }
    }
    return 0;
}

void main(){
    CreateRoom(&firstRoom, &lastRoom,1,1);
    printf("%d",(*firstRoom).number);
}

Standard warning: Please don't cast return of malloc http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc — niyasc, Apr 08 '15 at 06:18
@niyasc Thanks, I will try to take a look at it. But I don't know much about Programming — Tsz Fung Li, Apr 08 '15 at 06:26
@CraigAnderson, I thought that won't affact the result. But I'll return "new" because most people do so. — Tsz Fung Li, Apr 08 '15 at 06:26

score 1 · Accepted Answer · answered Apr 08 '15 at 06:20

1

if (*first=NULL){
            *first=new;
}

= is assignment operator. You should use == for comparison.

answered Apr 08 '15 at 06:20

niyasc

4,440
1
23
50

OMG, I never thought the compiler cannot catch this – Tsz Fung Li Apr 08 '15 at 06:25
@TszFungLi - if you use gcc you can compile with `-Wparentheses`. This would have caught this error. – Craig S. Anderson Apr 08 '15 at 06:31

score 0 · Answer 2 · answered Apr 08 '15 at 09:10

You shouldn't really bother about the last element. (If you need to traverse the list backward, you have to have a prev member in addition to next.) Now, if you want CreateRoom() to always add a new element at the end of the list, it should first traverse the whole list until it reaches the end of it —which it recognizes because of the NULL pointer— and then assign the new pointer to the place it has reached:

while (*first != NULL)
    first = &(*first)->next;
new->number = RoomNumber;
new->luxury = type;
new->next = *first;
*first = new;

Two points are worth being noted:

The assignment *first = new doesn't know if first is firstRoom or the next member of an actual element.
The while loop can be omitted to have new elements inserted at the beginning, or modified so as to have elements sorted the way you want.

Adding nodes inside a linked list using a function

2 Answers2