0

I am trying to use a bubble sort to sort a linked list. I can't just swap the values inside the nodes either. I've been drawing pictures trying to figure out how to do it myself without help, but I'm starting to get a head ache and can't figure out why this wont work.

void sort_ascending(struct node ** head){
  int x;
  struct node*temp;
  struct node*temp2;
  x = length(*head)+1;    //checks if more than one node is in the list
  if(x < 2){
    printf("1 or less\n");
    //free(temp);
    return;
  }
  printf("longer than 1\n");
  printf("%d %d\n", (*head)->val, (*head)->next->val);
  if((*head)->val > (*head)->next->val){
    printf("needs to sort!\n");
    temp = (*head)->next->next; //sets temp to the node after the two nodes being swapped
    printf("test1\n");
    temp2 = (*head); //sets temp2 to the node1
    printf("test2\n");
    *head = (*head)->next; //changes head to point at node2 instead of node1
    printf("test3\n");
    (*head)->next = temp2; //sets node2 to point to node1
    (*head)->next->next = temp; //sets node2 to point back into the list
    printf("test4\n");
    //free(temp);
  }
}

Right now I'm just trying to sort two nodes. After I can get this working I'll make it into a loop. For some reason, it isnt even sorting the first two elements.

Here are some of my other functions to help with understanding:

struct definition:

struct node {
int val;
struct node *next;

};

other functions:

void push(struct node ** headRef, int data){
struct node* newNode =  malloc(sizeof(struct node)); //alocates space on heap
printf("pushed node\n");
newNode->val = data;//sets data value
printf("%d\n", newNode->val);
newNode->next = *headRef; // The '*' to dereferences back to the real head
*headRef = newNode; // ditto

};

void print(struct node * head, int length){
int x = 0;
printf("tried to print\n");
//struct node*temp = head;

//while(head->next != NULL){
while (x < length + 1){
    printf("ran loop\n");
    printf("%d\n", head->val);
    printf("got number\n");
    head = head->next;
    x++;
}
printf("done with loop\n");

}

int main(){
char ans;
int num;
struct node *head = NULL;
do {
    do {
        printf("Enter a number: ");
        scanf("%d", &num);
        push(&head, num);//Can change to append for back

        printf("Do you want another num (y or n): ");
        scanf("%1s", &ans);
    } while (ans == 'y');

    printf("Sort ascending or descending (a or d)? ");
    scanf("%1s", &ans);
    if(ans == 'a') sort_ascending(&head);
    //else if(ans == 'd') sort_descending(&head);

    print(head, length(head));

    printf("Do you want to do this again (y or n)? ");
    scanf("%1s", &ans);
    if (ans == 'y') clear(&head);

} while (ans == 'y');


return 0;

}

int length(struct node* head){
int length = 0;
//struct node*temp = head;
printf("tried to find length\n");
while (head->next != NULL){
    length++;
    head = head->next;
}
printf("%d\n", length + 1);
return length;

}

14
  • How are you calling this function? How are you testing to see if this works? Commented Jun 6, 2014 at 7:00
  • after I have created two nodes, say one with a value of 5 and one of 6, I call this function to sort the nodes I have created. If it rearranges them, ill know it works Commented Jun 6, 2014 at 7:02
  • I know you understand how sorting works. :) I want to know 1) how you're constructing your nodes (show us your code), and 2) if you're inspecting values in a debugger or printing them out to screen to test if this works. Commented Jun 6, 2014 at 7:06
  • @WernerHenze While your malloc statement is true, the OP did state that he's only trying to sort the first two elements right now, and will add the loops later. Commented Jun 6, 2014 at 7:08
  • Thanks for the malloc info, I will remove them. Commented Jun 6, 2014 at 7:09

1 Answer 1

0

So, let's sum it up.

The function length is off by 1.

int length(struct node* head){
    int length = 0;
    while (head != NULL){
        ++length;
        head = head->next;
    }
    return length;
}

The function print is printing too much.

void print(struct node * head, int length){
    int x;
    for(x = 0; x < length; ++x){
        printf("%d\n", head->val);
        head = head->next;
    }
}

And your scanf is corrupting the memory. scanf with "%1s" is expecting a pointer to at least two chars, one to store and the trailing null byte. So you need to either provide two chars (char ans[2];) or, the better solution, just read one character as a character.

char ans;
scanf("%c", &ans);

Also, as said, don't cast the return value of malloc if you are programming in C.

If you wonder why I changed your postfix ++ (like x++) to prefix ++ (like ++x): I am a C++ programmer and for C++ it is recommended to prefer prefix ++ over postfix ++ for performance reasons (not relevant for int or pointers, but for complex types like iterators).

Sign up to request clarification or add additional context in comments.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.