Sam Barefoot
Sam Barefoot

Reputation: 13

Bubble Sort method not working with Linked List in C

I am trying to implement a bubble sort method into a linked list data structure, but when running it through a test harness, it doesn't do anything. Any suggestions?

Here is my source code:

void set_sort (set_t * the_set)
{
    assert (the_set);
    set_node_t *current;
    current = the_set->head;
    int sorted = 1;
    int x;
    for (x = 0; x < the_set->set_size; x++) {
        //we dont do it if this is the last value
        if (x + 1 == the_set->set_size) {
            continue;
        }
        if (current->data > current->next->data) {
            sorted = 0;
            int temp = &current->next->data;
            current->next->data = current->data;
            current->data = temp;
        }
        current = current->next;
    }
    if (!sorted) {
        set_sort (the_set);
    }

}

EDIT WITH HEADER FILE

#ifndef _set_h_
#define _set_h_

#include <stdbool.h>
#include "common.h"

/* Create a basic singly-linked list.*/
/*This code has been sourced from Mike Mcallistar, Assignment 5 Solutions*/

typedef struct _set_node_t {
        test_type_t *data;
        struct _set_node_t *next;
        struct _set_node_t *below;
} set_node_t;

/* the set itself keeps track of the head and the tail of the linked list */
typedef struct {

        int set_size;
        bool ready;
        set_node_t *head;
        set_node_t *tail;
        int set_level;
} set_t;


bool set_init(set_t *the_set);
void set_destroy(set_t *the_set);
bool set_add(set_t *the_set, test_type_t *item_to_add);
bool set_delete(set_t *the_set, test_type_t *item_to_remove);
bool set_find( set_t *the_set, test_type_t *item_to_find);
void set_sort(set_t *the_set);
void set_enumerate(set_t *the_set);
#endif

Upvotes: 1

Views: 120

Answers (1)

Philip Couling
Philip Couling

Reputation: 14913

Something seems very wrong with this.

int temp = &current->next->data;     // Assignes a pointer into temp
current->next->data = current->data; // Copies current into next
current->data = temp;                // Copies the pointer into data 

This is unlikely to do nothing. It's quite likely to corrupt your data.

Could it be as simple as to change the first of these lines to:

int temp = current->next->data;

Edit

Cleaning up your code a little I get to this:

void set_sort(set_t *the_set)
{
    assert(the_set);
    int sorted;
    int x;
    do {
        set_node_t *current = the_set->head;
        sorted = 1;
        for( x = 0; x < the_set->set_size - 1; x++){
            if(current->data > current->next->data){
                sorted = 0;
                int temp = current->next->data;
                current->next->data = current->data;
                current->data = temp;
            }
            current = current->next;
        }
    }
    while (!sorted);
}

Removing the use of unnecessary recursion removes the risk of causing a stack overflow. Removing the continue makes the code marginally quicker (I believe). Removing the spurious use of the pointer should fix your code.

If your code isn't fixed by this then then you will need to post the definition of set_node_t, its possible your comparison is not working (if (current->data > current->next->data)).

Edit 2

As comments and updated question has now pointed out you need to perform your comparison on the data itself and not the pointer to the data.

if(*(current->data) > *(current->next->data)){

Upvotes: 1

Related Questions