CODEWITHSUNDEEP
c++pointersmax
Maha K
Maha K

Reputation: 1

Findmax() method returns second highest value when first element of array is the highest

This findMax function returns max value from the array only if it is located at some index other than first index. I don't understand why because my findMin function that has almost the same code works perfectly fine.

void findMax(int array[5])
{
    maximum = array; 
    for (i = 0; i < 5; i++)
    {
    if (*(array+i) > *maximum)
        *maximum = *(array+i);
    }
    cout<<"Maximum element in the array is "<< *maximum << "\n" ;
}

This is my findMin fun that is working fine.

void findMin(int array[5])
{
    minimum = array;    
    for (i = 0; i < 5; i++)
    {
    if (*(array+i) < *minimum)
        *minimum = *(array+i);
    }
    cout<<"Minimum element in the array is "<< *minimum <<"\n";
}

Upvotes: 0

Views: 443

Answers (3)

Thomas
Thomas

Reputation: 182073

The other answers have described how to do this more cleanly in C++. But to point out the actual bug: it's in this line.

    *maximum = *(array+i);

You're not reassigning the maximum pointer to point to the maximum element, but rather you're never changing the pointer, but changing the value inside the array where maximum points to (i.e. array[0]). You meant this instead:

    maximum = array + i;

The same issue is present in your findMin function as well.

Upvotes: 1

user11050329
user11050329

Reputation:

There are minmax_element tools for finding maximum and minimum, this is the most optimal variant of solving your problem - see the definition of the StdMinMax function. But if you want to implement the logic yourself, I gave an example of a function, see the definition of the MinMax function

#include <iostream>
#include <algorithm>


void StdMinMax(int* arr, const unsigned int size)
{
    std::pair<int*, int*> bounds = std::minmax_element(arr, arr + size); // or use auto bounds = ... ore auto [max, min] = ...

    std::cout << "min : " << *bounds.first << " max : " << *bounds.second << std::endl;

}


void MinMax(int* arr, const unsigned int size)
{
    std::cout << "Find max : " << std::endl;

    auto currentMax = *arr;
    for (int i = 1 ; i < size; ++i)
    {
        if (arr[i] > currentMax)
        {
            currentMax = arr[i];
        }
    }

    std::cout << "Max : " << currentMax << std::endl;

    std::cout << "Find min : " << std::endl;

    auto currentMin = *arr;
    for (int i = 1 ; i < size; ++i)
    {
        if (arr[i] < currentMax)
        {
            currentMin = arr[i];
        }
    }

    std::cout << "Min : " << currentMin << std::endl;
}


int main()
{
    const unsigned int size{5};

    int array[size]{1, 3, 4, -11, 77};

    StdMinMax(array, size);

    MinMax(array, size);

    return 0;
}

Upvotes: 0

Harun Tucakovic
Harun Tucakovic

Reputation: 146

First of all, as one of the comments on your question said this is mostly C way of doing things. In C++ you should use std::vector, std::min_element and std::max_element. It's easier and safer to use them instead of doing everything manually by yourself.

But, if you really want to do it yourself, try this code out, it should work:

void findMax(int array[])
{
    maximum = array;
    for (int i = 1; i < 5; i++)
    {
        if (*(array + i) > *maximum)
            maximum = (array + i);
    }
    cout << "Maximum element in the array is " << *maximum << "\n";
}

void findMin(int array[])
{
    minimum = array;
    for (int i = 1; i < 5; i++)
    {
        if (*(array + i) < *minimum)
            minimum = (array + i);
    }
    cout << "Minimum element in the array is " << *minimum << "\n";
}

This should work assuming that minimum and maximum are globally declared like this: 

int * maximum;
int * minimum;

Upvotes: 0

Related Questions