snippetcppMinor
Modified bucket sort
Viewed 0 times
modifiedsortbucket
Problem
The code runs fine and the algorithm is much faster than insertion sort. I built a modified bucket sort around the parameters given in the HW to sort a vector of ints with 50 iterations and 40000 items.
Being new and not familiar with bucket sorts I haven't found any code examples quite like this one. So my question is about my style and possible room for optimization. How can I streamline this algorithm?
Being new and not familiar with bucket sorts I haven't found any code examples quite like this one. So my question is about my style and possible room for optimization. How can I streamline this algorithm?
void intVectSort2(vector &V, int M)
{
// declare size for U and V
int uSize = ((M * 2) + 1);
int vectSize = V.size();
// allocate memory for U array and initialize to zeros
int *U = new int[uSize];
for (int i = 0; i < uSize; i++)
U[i] = 0;
// step through M values and increment U when V is equal to M
for (int j = 0; j < vectSize; j++)
{
int val = 0;
int count = 0;
int element = 0;
int iterator = M;
val = V[j];
while (val != iterator)
iterator--;
element = iterator + M;
count = U[element];
count++;
U[element] = count;
}
//load confirmed values in order back into V
int index = 0;
int currentSize = 0;
for (int k = 0; k < uSize; k++)
{
int value = 0;
int counter = 0;
value = U[k];
if (value != 0)
{
while (counter < value)
counter++;
currentSize += counter;
for (index; index < currentSize; index++)
V[index] = k - M;
}
}
// free dynamic memory
delete [] U;
}Solution
Pointless loops
You have a couple of loops that do nothing but waste time:
That loop does the same thing as:
Similarly, this loop:
is the same as:
Way too complicated
Your code that fills the buckets is confused and complicated:
It should really be a one line loop:
Note that this uses buckets in the range 0..M, whereas yours uses buckets in the range M..2M for no good reason.
Similarly, your bucket unpacking loop could be simplified to:
This assumes that you allocated
You have a couple of loops that do nothing but waste time:
while (val != iterator)
iterator--;That loop does the same thing as:
iterator = val;Similarly, this loop:
while (counter < value)
counter++;is the same as:
counter = value;Way too complicated
Your code that fills the buckets is confused and complicated:
for (int j = 0; j < vectSize; j++)
{
int val = 0;
int count = 0;
int element = 0;
int iterator = M;
val = V[j];
while (val != iterator)
iterator--;
element = iterator + M;
count = U[element];
count++;
U[element] = count;
}It should really be a one line loop:
for (int j = 0; j < vectSize; j++) {
U[V[j]]++
}Note that this uses buckets in the range 0..M, whereas yours uses buckets in the range M..2M for no good reason.
Similarly, your bucket unpacking loop could be simplified to:
int index = 0;
for (int i = 0; i <= M; i++) {
for (int j = 0; j < U[i]; j++) {
V[index++] = i;
}
}This assumes that you allocated
U to hold M+1 elements, to allow for values in the range 0..M.Code Snippets
while (val != iterator)
iterator--;iterator = val;while (counter < value)
counter++;counter = value;for (int j = 0; j < vectSize; j++)
{
int val = 0;
int count = 0;
int element = 0;
int iterator = M;
val = V[j];
while (val != iterator)
iterator--;
element = iterator + M;
count = U[element];
count++;
U[element] = count;
}Context
StackExchange Code Review Q#161376, answer score: 2
Revisions (0)
No revisions yet.