user14880070
user14880070

Reputation:

i want to make sure that my linked list work

Is this a right way to do a linked list ? I am having a problem in a big school project and now i want to make sure that this is true.

void addnode(int a){
    struct house* tmp = houses[i].next;
    while (tmp != NULL) {
        tmp = tmp->next;
    }
    tmp = (struct house*)malloc(sizeof(struct house));
    tmp->id=a;
    tmp->next=NULL;
}

i figured out that the error can be in other parts of the code. Now i will share the parts i suspect i hope you can help me. houses[i] is an array of linked lists. if houses[i].id==-1 it is empty

struct house get_house_byid(int id) {
    for (int i = 0; i < 1000; i++) {
        if (houses[i].id != -1) {
            if (houses[i].id == id) {
                return houses[i];
            }
            if (houses[i].next != NULL) {
                struct house* tmp = houses[i].next;
                while (tmp != NULL) {
                    if (tmp->id == id) {
                        return *tmp;
                    }
                    tmp = tmp->next;
                }
            }
        }
    }
    struct house housep;
    housep.id = -1;
    return housep;//if it cant find that id it returns housep
}

Upvotes: 1

Views: 57

Answers (1)

Craig Estey
Craig Estey

Reputation: 33601

There may be other issues with your code that is not shown, but there are issues with addnode:

  1. addnode does not set the head of the list (i.e. houses[i].next).
  2. Thus, the newly added node is never connected to anything [and is a memory leak].
  3. Ignoring the [obvious] typo/syntax error: void addnode{int a} instead of void addnode(int a).
  4. The loop on tmp discards the pointer to the tail of the list. We need a separate variable (e.g. prev).
  5. Note that i is global. That's fine, but the function would be cleaner if i was an argument to addnode instead.
  6. Don't cast the return of malloc: Do I cast the result of malloc?

Here's is some refactored code. It is annotated:

void
addnode(int i,int a)
{
    struct house *tmp;
    struct house *prev;

    // find the tail of the list
    prev = NULL;
    for (tmp = houses[i].next;  tmp != NULL;  tmp = tmp->next)
        prev = tmp;

    // allocate the new node
    tmp = malloc(sizeof(*tmp));
    tmp->id = a;
    tmp->next = NULL;

    // append to the tail of the [non-empty] list
    if (prev != NULL)
        prev->next = tmp;

    // add to front of the empty list
    else
        houses[i].next = tmp;
}

Upvotes: 1

Related Questions