patterncppMinor
Heapsort is not modular
Viewed 0 times
heapsortmodularnot
Problem
For the time being, I am not interested in any recursive solution. The code is not modularized; the whole body is inside the main function.
#include
#include
/*This code is written by NF between any time of 120711 and 210711;modified at 231211*/
int main()
{
int length=0,min_heapified=0,start_length=0,index=0;
cout>length;
int array[length];
for(int i=0;i0);
int swap=array[0];
array[0]=array[--length];/*modification done at this point to avoid keeping the sorted elements into another array;and swapping done between the 0th and last element*/
array[length]=swap;
cout0);
return 0;
}Solution
int length=0,min_heapified=0,start_length=0,index=0;I prefer one declaration per line because it's easier to read and find the type of variables. Furthermore,
min_heapified is only used inside the do-while loop, so declare it there:do {
int min_heapified = 0;
do {
...Change
if(x<=(length-1))to
if (x < length)It's more common and simple.
Instead of
int swap=array[left];
array[left]=array[root];
array[root]=swap;you should create a
swap function. Maybe it already exists in a library.int swap(array, index, rootIndex) {
int swap = array[index];
array[index] = array[rootIndex];
array[root] = swap;
}If I'm right you could extract the following to a new function:
if(array[left]<array[root])
{
int swap=array[left];
array[left]=array[root];
array[root]=swap;
min_heapified=1;
}It looks for me that the other
if do the same. The only difference is the used index, so I'd write something like this:int swapIfLess(array, index, rootIndex) {
if (array[index] < array[rootIndex]) {
swap(array, index, rootIndex)
return 1;
}
return 0;
}if (left < length) {
if (swapIfLess(array, left, root)) {
min_heapified=1;
}
}
if (right < length) {
if (swapIfLess(array, right, root)) {
min_heapified=1;
}
}The
i variable looks unnecessary:int root=i;Use the
root variable in the for loop:for (int root = 0; root < length; root++)Maybe you should rename it to
rootIndex.Extract more functions. The
for loop (inside the do-while loops) looks as a good candidate for this. Additionally, you should have a readInputArray function and a printResults function too.Code Snippets
int length=0,min_heapified=0,start_length=0,index=0;do {
int min_heapified = 0;
do {
...if(x<=(length-1))if (x < length)int swap=array[left];
array[left]=array[root];
array[root]=swap;Context
StackExchange Code Review Q#6196, answer score: 7
Revisions (0)
No revisions yet.