CODEWITHSUNDEEP
c++
Chris Kraszewski
Chris Kraszewski

Reputation: 690

Finding biggest number in array returns max int value

I want to find the biggest and lowest numbers in array using a function. Here's my code:

#include <stdio.h>

void findLowHigh(int* numbers, int size, int* min, int* max) {
    for(int i = 0; i < size; i++) {
        if (*min > numbers[i]) *min = numbers[i];
        else if (*max < numbers[i]) *max = numbers[i];
    }
}

int main() {
    printf("##### Find lowest and highest number in collection #####");
    printf("\n Array checked: [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ]");
    int min, max;
    int numbers1[10] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };
    findLowHigh(numbers1, 10, &min, &max);
    printf("\nMin: %d\nMax: %d", min, max);

    printf("\n Array checked: [ 10, 9, 8, 7, 6, 5, 4, 3, 2, 1 ]");
    int numbers2[10] = { 10, 9, 8, 7, 6, 5, 4, 3, 2, 1 };
    findLowHigh(numbers2, 10, &min, &max);
    printf("\nMin: %d\nMax: %d", min, max);

    return 0;
}

And there's output:

##### Find lowest and highest number in collection #####
Array checked: [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ]
Min: 1
Max: 32767
Array checked: [ 10, 9, 8, 7, 6, 5, 4, 3, 2, 1 ]
Min: 1
Max: 32767
Process finished with exit code 0

I'm guessing there's something wrong with pointers, but what is it?

Upvotes: 0

Views: 182

Answers (5)

StoryTeller - Unslander Monica
StoryTeller - Unslander Monica

Reputation: 170055

if (*min > numbers[i]) Assume *min already contains a legal value.

But you don't initialize the integer whose address you pass. A possible fix (after including limits.h (or climits, if you really are compiling C++) ):

int min=INT_MAX;
int max=INT_MIN;

As Nathan Oliver suggested in the comments. A more robust solution would be to set min and max to the first element of the array.

if (min && max && numbers && size > 0) {
  *min = numbers[0];
  *max = numbers[0];
}

Upvotes: 1

Fran&#231;ois Andrieux
Fran&#231;ois Andrieux

Reputation: 29023

int min, max; are uninitialized, they can contain any value. You should initialize at the beginning of findLowHigh. It's important to reset them before you call findLowHigh or past results will have an impact on future results.

You can simply set them both to the first element of numbers and skip that element in your loop. Make sure to check that size is non-zero first.

void findLowHigh(int* numbers, int size, int* min, int* max) 
{
    if(size > 0)
    {
        *min = numbers[0];
        *max = numbers[0];

        for(int i = 1; i < size; i++) {
            if (*min > numbers[i]) *min = numbers[i];
            if (*max < numbers[i]) *max = numbers[i]; // Also, remove this else
        }
    }
}

Edit : You'll have to decide what should happen in case size is zero. What is the min and max of an empty list?

Upvotes: 1

marcinj
marcinj

Reputation: 49976

You should initialize your min/max:

int min, max;

like:

int min=INT_MAX, max=INT_MIN;

More in c++ style you would use #include <limits> and:

int min = std::numeric_limits<int>::max();
int max = std::numeric_limits<int>::min();

[edit]

You should also (as it was in comments) remove the else in your logic from:

if (*min > numbers[i]) *min = numbers[i];
    else if (*max < numbers[i]) *max = numbers[i];

otherwise you may never find a max value in your results.

Upvotes: 2

Ali Camilletti
Ali Camilletti

Reputation: 135

When you initialize your variables like int min, max; you leave them blank instead of a number, this means that min and max is automatically set as undefined numbers.

Upvotes: 0

Dipti Shiralkar
Dipti Shiralkar

Reputation: 570

As you are comparing with value of min and max, before passing the value to function you should initialise the values min=INT_MAX and max=INT_MIN.

Upvotes: 1

Related Questions