0

I am new to coding and I am unable to see what is wrong with this Logic. I am unable to get the desired output for this program. The Question is to find the minimum and maximum elements of an array.

The idea is to create two functions for minimum and maximum respectively and have a linear search to identify the maximum as well as a minimum number.

#include <iostream>
#include<climits>
using namespace std;
void maxElement(int a[], int b)
{
    // int temp;
    int maxNum = INT_MIN;
    for (int i = 0; i < b; i++)
    {
        if (a[i] > a[i + 1])
        {
             maxNum = max(maxNum, a[i]);
        }
        else
        {
             maxNum = max(maxNum, a[i+1]);
        }
        // maxNum = max(maxNum, temp);
    }
    // return maxNum;
    cout<<maxNum<<endl;

    
}
void minElement(int c[], int d)
{
    // int temp;
    int minNum = INT_MAX;
    for (int i = 0; i < d; i++)
    {
        if (c[i] > c[i + 1])
        {
            minNum = min(minNum,c[i+1]);
        }
        else 
        {
            minNum = min(minNum,c[i]);
        }
        // minNum = min(minNum, temp);
    }
    // return minNum;
    cout<<minNum<<endl;
}
int main()
{
    int n;
    cin >> n;
    int arr[n];
    for (int i = 0; i < n; i++)
    {
        cin >> arr[i];
    }
   
   minElement(arr,n);
   maxElement(arr,n);

   

    return 0;
}
Evg
  • 25,259
  • 5
  • 41
  • 83
Parth Tiwari
  • 11
  • 2
  • 2
    why do you consider `a[i]` and `a[i+1]` in one iteration? `i+1` is not a valid index in the last iteration – 463035818_is_not_an_ai Sep 06 '21 at 10:57
  • 1
    also `int arr[n];` is not standard C++. [Why aren't variable-length arrays part of the C++ standard?](https://stackoverflow.com/questions/1887097/why-arent-variable-length-arrays-part-of-the-c-standard) – 463035818_is_not_an_ai Sep 06 '21 at 10:58
  • You only compare adjacent numbers before deciding if a number is a new min or max. You need to compare the new number to the existing min or max instead of to its adjacent number. – L. Scott Johnson Sep 06 '21 at 10:59

1 Answers1

1

You are already comparing each element to the current max / min. It is not clear why in addition you compare to adjacent elements. Trying to access a[i+1] in the last iteration goes out of bounds of the array and causes undefined behavior. Just remove that part:

void maxElement(int a[], int b)
{
    // int temp;
    int maxNum = INT_MIN;
    for (int i = 0; i < b; i++)
    {
             maxNum = max(maxNum, a[i]);
    }
    cout<<maxNum<<endl;
}

Similar for the other method.

Note that

int n;
cin >> n;
int arr[n];

is not standard C++. Variable length arrays are supported by some compilers as an extension, but you don't need them. You should be using std::vector, and if you want to use c-arrays for practice, dynamically allocate the array:

int n;
cin >> n;
int* arr = new int[n];

Also consider to take a look at std::minmax_element, which is the standard algorithm to be used when you want to find the min and max element of a container.

Last but not least you should seperate computation from output on the screen. Considering all this, your code could look like this:

#include <iostream>
#include <algorithm>

std::pair<int,int> minmaxElement(const std::vector<int>& v) {
    auto iterators = std::minmax_element(v.begin(),v.end());
    return {*iterators.first,*iterators.second};
}
int main()
{
    int n;
    std::cin >> n;
    std::vector<int> input(n);
    for (int i = 0; i < n; i++)
    {
        std::cin >> input[i];
    }
   
    auto minmax = minmaxElement(input);
    std::cout << minmax.first << " " << minmax.second;
}

The method merely wraps the standard algorithm. It isnt really needed, but I tried to keep some of your codes structure. std::minmax_element returns a std::pair of iterators that need to be dereferenced to get the elements. The method assumes that input has at least one element, otherwise dereferencing the iterators is invalid.

463035818_is_not_an_ai
  • 109,796
  • 11
  • 89
  • 185