CODEWITHSUNDEEP
c++
Peter Petrov
Peter Petrov

Reputation: 23

The three biggest numbers in array

#include<iostream>
using namespace std;
int main()
{
    int s;
    cin>>s;
    int t=3;
    int maxValue,imax[t],maxIndex,arr[s];
    for(int i=0; i<s; i++){
        cin>>arr[i];
    }
    maxValue=arr[0];
    for(int i=0;i<s;i++){
        if(arr[i]>maxValue){
             maxValue=arr[i];
             imax[0] = i;
        }
    }
    maxValue=arr[0];
    for(int i=0;i<s;i++){
        if (i == imax[0]) { continue; }
        if(arr[i]>maxValue){
            maxValue=arr[i];
            imax[1] = i;
        }
    }
    maxValue=arr[0];
    for(int i=0;i<s;i++){
        if (i == imax[0]) { continue; }
        if (i == imax[1]) { continue; }
        if(arr[i]>maxValue){
            maxValue=arr[i];
            imax[2] = i;
        }
    }
    cout<<"First biggest number:"<<arr[imax[0]]<<"\n";
    cout<<"Second biggest number:"<<arr[imax[1]]<<"\n";
    cout<<"Third biggest number:"<<arr[imax[2]];
    return 0;
}

This program must return tree numbers which is biggest in this arraybut , i do not know why when I introduce as example five numbers (121,34,56,67,545) and the compiler was return 545 and then crash. Thank you in advance for the answer.

Upvotes: 1

Views: 179

Answers (2)

CaptainSegFault
CaptainSegFault

Reputation: 82

The problem is that before iterating the loop, you first set the maxValue to be the first element in the array. The imax only gets updated whenever there is at least one element greater than the current maxValue. However, if the first element is somehow the maxValue you are looking for, then the imax never gets set, which would be uninitialized causing segmentation fault at the end.

In your code, after finding the largest element 545, the second largest element was never found, since 121 is the first element in the array. Hence after printing out 545, imax[1] is uninitialized and the program crashes.

Upvotes: 2

Oleksandr Senkovych
Oleksandr Senkovych

Reputation: 41

You use uninitialized array values in lines 

cout<<"First biggest number:"<<arr[imax[0]]<<"\n";
cout<<"Second biggest number:"<<arr[imax[1]]<<"\n";
cout<<"Third biggest number:"<<arr[imax[2]];

If there are less than 3 different numbers in input, some imax array elements will not be initialized. Also if input array is empty, imax will not be initialized at all.

Therefore in expression arr[imax[1]] you read element of arr with index, which was not initialized and can be some very big number. It can be fixed if you declare iarr as

int imax[t] = {};

This will zero-initialize all elements of array and will prevent crashing.

Your program also doesn't check number of elements in input array, so if there are less than three input numbers arr[2] will also print uninitialized value.

Here's proper solution using STL algorithms and std::vector. It works with any number of t - you can easily change it to print largest 10 numbers. It is also memory efficient - it does not need to store whole input array so you can process large inputs with it.

#include <algorithm>
#include <iostream>
#include <vector>

int main() {
  int s;
  std::cin >> s;

  unsigned t = 3;
  std::vector<int> max_numbers;
  max_numbers.reserve(t + 1);

  for (int i = 0; i < s; ++i) {
    int number;
    if (std::cin >> number) { //Check basic input errors
      max_numbers.push_back(number); // Add number to top-3 list

      // Sort elements in descending order
      std::sort(max_numbers.begin(), max_numbers.end(), std::greater<int>());

      // Remove duplicates
      max_numbers.erase(std::unique(max_numbers.begin(), max_numbers.end()),
                        max_numbers.end());

      // Remove excess elements
      if (max_numbers.size() > t) {
        max_numbers.resize(t);
      }
    }
  }

  std::cout << "Biggest " << t << " numbers are" << std::endl;
  for (int i : max_numbers) {
    std::cout << i << std::endl;
  }
}

Upvotes: 1

Related Questions