0
votes

I was trying to figure out a code about heap sort using binary tree that I saw on stackoverflow.com,

here is the code:

//Heap Sort using Linked List
//This is the raw one
//This getRoot function will replace the root with number in the last node, after the main prints the largest number in the heap
//The heapSort function will reconstruct the heap
//addNode function is as same as in binary search tree
//Note addNode and heapSort are recursive functions
//You may wonder about the for loop used in main, this actually tells the depth of the tree (i.e log base2 N)
//With this value these functions find where to trverse whether left or right(direction), with help of macro GETBIT (0-left,1-right)


#include<stdio.h>
#include<malloc.h>
#include<stdlib.h>

#define GETBIT(num,pos) (num >> pos & 1)

struct node
{
    int data;
    struct node *left;
    struct node *right;
};
typedef struct node node;

int nodes;
node *first, *tmp, *current;

void addNode(node *,node *,int);
void swap(int *, int *);
void getRoot(node *, int);
void heapSort(node *);

int main()
{
    int num;
    int cont,i,j;

    while(1)                                                //It gets number from user if previous value is non zero number
    {
        printf("Enter a number\n");
        scanf("%d",&num);
        if(!num)                                            //i'm using 0 as terminating condition to stop adding nodes
            break;                                          //edit this as you wish

        current = (node *)malloc(sizeof(node));
        if(current ==  0)
            return 0;

        current->data = num;
        nodes++;

        for(i=nodes,j=-1; i; i >>= 1,j++);

        if(first == 0)
        {
            first = current;
            first->left = 0;
            first->right = 0;
        }
        else
            addNode(first,first,j-1);

        printf("Need to add more\n");

    }
    printf("Number of nodes added : %d\n",nodes);

    while(nodes)
    {
        printf(" %d -> ",first->data);                                        //prints the largest number in the heap

        for(i=nodes,j=-1; i; i >>= 1,j++);                                    //Updating the height of the tree
        getRoot(first,j-1);
        nodes--;

        heapSort(first);
    }       

    printf("\n\n");
    return 0;
}

void swap(int *a,int *b)
{
    *a = *a + *b;
    *b = *a - *b;
    *a = *a - *b;
}

void addNode(node *tmp1,node *parent, int pos)
{
    int dirxn = GETBIT(nodes,pos);                                   // 0 - go left, 1 - go right

    if(!pos)
    {
        if(dirxn)
            tmp1->right = current;
        else
            tmp1->left = current;

        current->left = 0;
        current->right = 0;

        if(current->data > tmp1->data)
            swap(&current->data, &tmp1->data);
    }

    else
        if(dirxn)
            addNode(tmp1->right,tmp1,pos-1);
        else
            addNode(tmp1->left,tmp1,pos-1);

    if(tmp1->data > parent->data)
        swap(&parent->data,&tmp1->data);
}

void getRoot(node *tmp,int pos)
{
    int dirxn;

    if(nodes == 1)
        return ;

    while(pos)
    {
        dirxn = GETBIT(nodes,pos);

        if(dirxn)
            tmp = tmp->right;
        else
            tmp = tmp->left;
        pos--;
    }

    dirxn = GETBIT(nodes,pos);

    if(dirxn)
    {
        first->data = tmp->right->data;
        free(tmp->right);
        tmp->right = 0;
    }
    else
    {
        first->data = tmp->left->data;
        free(tmp->left);
        tmp->left = 0;
    }
}

void heapSort(node *tmp)
{
    if(!tmp->right && !tmp->left)
        return;

    if(!tmp->right)
    {
        if(tmp->left->data > tmp->data)
            swap(&tmp->left->data, &tmp->data);
    }
    else
    {
        if(tmp->right->data > tmp->left->data)
        {
            if(tmp->right->data > tmp->data)
            {
                swap(&tmp->right->data, &tmp->data);
                heapSort(tmp->right);
            }
        }           
        else
        {
            if(tmp->left->data > tmp->data)
            {
                swap(&tmp->left->data, &tmp->data);
                heapSort(tmp->left);
            }
        }
    }
}

I don't really understand the right shift operator so I tried to replace for(i=nodes,j=-1; i; i >>= 1,j++); with

int o, j=0;
for(int i=1;;i=pow(2, j)){
    if(nodes<i){
        o = j-1;
        break;
    }
    j+=1;
}

but I don't understand dirxn = GETBIT(nodes,pos);

my question is what does this do? and can anyone tell me what should I do to replace this with something without a shift operator?

any help will be greatly appreciated

The >>= is a right-shift operator, not a left-shift operator. It effectively divides by 2. Your loop condition is emphatically not the same as the original. The original contains a test i != 0 (abbreviated i); yours has no condition. That's worrying.Jonathan Leffler
sorry, I fixed the right shift. the loop is working fine.Someone's name
I observe that the macro #define GETBIT(num,pos) (num >> pos & 1) is riskily written. It should be at minimum #define GETBIT(num,pos) ((num) >> (pos) & 1) to ensure that if num or pos is some sort of arithmetic expression, the condition is the same. I think it would benefit from another set of parentheses: #define GETBIT(num,pos) ((num) >> (pos)) & 1) to clarify that the & applies to the result of the shift rather than to (pos) per se. Given the actual usage, it is OK, but general-purpose macros should use enough parentheses to ensure that they are safe against plausible misuse.Jonathan Leffler
The swap() function void swap(int *a,int *b) { *a = *a + *b; *b = *a - *b; *a = *a - *b; } is dubious. Given big enough values, it will (wholly unnecessarily) incur signed integer overflow leading to undefined behaviour. It should use a temporary variable: void swap(int *a,int *b) { int t = *a; *a = *b; *b = t; } which uses 4 pointer dereferences and 3 assignments, contrasted with 9 pointer dereferences, 3 assignments and 3 (potentially overflowing) arithmetic operations.Jonathan Leffler
@jak: And what if a and b point to the same object? Don't try to be clever with swap. It's error-prone and much slower than necessary. It only made sense, if it ever did, on CPUs without multiple registers.rici