Reputation: 3255
So, I'm just working on C code, particularly a function which accepts 3 arguments: an array, the size of the array, and the number of max elements you want returned.
Here's my code:
int* findMaxElements(int base_array[],int size_of_base_array, int number_of_elements_to_find);
int main( void )
{
printf("Find Max Values in an Array\n\n");
// Set up array
int kinch[6] = {1,2,3,4,5,6};
// Pass to function and get a pointer to new array filled with only the max elements
int *given = findMaxElements(kinch,6,3);
for(int i = 0; i < 3; i++)
{
printf("\nMax Value = %d\n", *(given + i));
}
return 0;
}
int* findMaxElements(int base_array[],int size_of_base_array, int number_of_elements_to_find)
{
// Set up all initial variables
int i,k,c,position;
int maximum = 0;
int returnArray[100];
/*Actual Algorythm */
for(i = 0; i < number_of_elements_to_find; i++)
{
// Get the max value in the base array
for(k = 0; k < size_of_base_array; k++)
{
if(base_array[k] > maximum)
{
maximum = base_array[k];
}
}
// Find the position of the max value
for(position = 0; position < size_of_base_array; position++)
{
if(base_array[position] == maximum)
{
break;
}
}
// Delete the maximum value from the array and shift everything
for(c = position - 1; c < size_of_base_array - 1; c++)
{
base_array[c] = base_array[c+1];
}
// Reduce the size of the array
size_of_base_array -= 1;
// Push max value into return array
returnArray[i] = maximum;
// Reset max value
maximum = 0;
}
return returnArray;
}
I have a feeling somewhere in the function something goes wrong.
// Set up array
int kinch[6] = {1,2,3,4,5,6};
// Pass to function and get a pointer to new array filled with only the max elements
int *given = findMaxElements(kinch,6,3);
for(int i = 0; i < 3; i++)
{
printf("\nMax Value = %d\n", *(given + i));
}
This should output the numbers 6, 5, and 4, because they are the three largest in the array, however the output I get is always 6, 6, and 6. What's wrong with it?
Upvotes: 0
Views: 999
Reputation: 341
The whole approach to find the Kth largest element is not efficient and elegant. I will suggest you to modify your algorithm, although with above suggestions it will work fine, but it's not good way to solve this problem.
I will suggest you to look into below link to modify your algorithm http://www.geeksforgeeks.org/k-largestor-smallest-elements-in-an-array/
Upvotes: 1
Reputation: 46365
This may not be your only problem, but in the lines
for(c = position - 1; c < size_of_base_array - 1; c++)
{
base_array[c] = base_array[c+1];
}
You copy the element at [c+1]
(which is the maximum) to [c]
- so you keep finding the max...
You should start the loop with c = position
, not c = position - 1
.
And add keyword static
in front of the array you use to store the return values, so they remain valid (this is one way to address the issue that Jonathan Leffler identified).
Upvotes: 2
Reputation: 753515
One problem is that you are returning a pointer to a local variable, returnArray
, in the function. You can't do that reliably — it leads to undefined behaviour.
There may well be other problems too, but that's enough to be a show-stopper on its own.
Upvotes: 2