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(¤t->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
>>=
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 testi != 0
(abbreviatedi
); yours has no condition. That's worrying. – Jonathan Leffler#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 ifnum
orpos
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 Lefflerswap()
functionvoid 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 Lefflera
andb
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