2

I'm doing a program that finds the max value in a array. I done it but I found a strange bug.

#include<iostream>

using namespace std;

int main() {
    int n; //input number of elements in 
    cin >> n;
    int arr[n];
    for (int i = 0; i < n; i++) {
        cin >> arr[i]; //input array's elements
    } int max_value = arr[0];
    for (int i = 1; i <= n; i++) {
        if (arr[i] > max_value) {
            max_value = arr[i];
        }
    } cout << max_value;
    return 0;
}

When I put 5 as first line for the number of elements and 2, 7, 6, 8, 9 as the elements of the array. It returns 16 instead of 9. Please help

Avry G.
  • 39
  • 5
  • 5
    Arrays in C++ start at 0, not 1. You're iterating to one-past-the-end, which is [undefined behavior](https://stackoverflow.com/questions/2397984/undefined-unspecified-and-implementation-defined-behavior) – Silvio Mayolo Aug 31 '22 at 01:38
  • 5
    Variable length arrays are not standard C++ so this code is not portable. Whitespace is free, you should consider using some. Things like `} int max_value = arr[0];` give me a headache. – Retired Ninja Aug 31 '22 at 01:41
  • 3
    `int arr[n];` - whichever C++ textbook showed you to do this -- you need to throw it away immediately, and get a different C++ textbook. If you copied that off some web site, don't visit that web site any more. If you saw this in some clown's Youtube video, unsubscribe from that channel, you're not learning proper C++. This is not standard C++, and many C++ compilers will refuse to compile this. – Sam Varshavchik Aug 31 '22 at 01:41
  • See also [std::max_element](https://en.cppreference.com/w/cpp/algorithm/max_element) – David C. Rankin Aug 31 '22 at 02:07

3 Answers3

2

In Arrays the first index starts with 0 and ends in n - 1 assuming the array is of length n

so when looping from i = 1 to i <= n. n is now larger than n - 1. the solution would be to start from 0 and end at i < n hence:

#include<iostream>

using namespace std;

int main() {
    int n; //input number of elements in 
    cin >> n;
    int arr[n];
    for (int i = 0; i < n; i++) {
        cin >> arr[i]; //input array's elements
    } int max_value = arr[0];
    for (int i = 0; i < n; i++) {
        if (arr[i] > max_value) {
            max_value = arr[i];
        }
    }
    cout << max_value;
    return 0;
}

you could also use the std::max function like so:

for(int i = 0; i < n; i ++) {
   max_value = max(max_value, arr[i]);
}
Baraa Halabi
  • 424
  • 4
  • 8
  • 2
    Worth also noting comments 2 and 3 under the main question in your answer. `int arr[n];` with `int n;` isn't valid C++, it's a non-standard extension. – David C. Rankin Aug 31 '22 at 02:09
1

in your second for do this

for (int i = 1; i < n; i++) {
    if (arr[i] > max_value) {
        max_value = arr[i];
    }

delete '=' from i <= n because i is index which start from 0

and instead of this

int arr[n];

do this

int *arr = new int[n];
  • 1
    `int arr* = new int[n];` statement is not valid, it should be `int *arr = new int[n];`. – H.S. Aug 31 '22 at 06:58
  • And better use `std::vector` in this case. It can be accessed like an array, if you do not like `push_back`. If you insist on C arrays, use `make_unique` in its array form. – Sebastian Aug 31 '22 at 07:00
  • Your answer could be improved with additional supporting information. Please [edit] to add further details, such as citations or documentation, so that others can confirm that your answer is correct. You can find more information on how to write good answers [in the help center](/help/how-to-answer). – Community Sep 05 '22 at 03:27
1

The other posts already pointed out problem in your code.

You should be aware of that int arr[n]; is not permitted in standard C++.
[GCC and CLANG compiler support it in C++ as an extension]

An alternative is to allocate memory dynamically:

    int *arr = new int[n];

and to find maximum value you can use std::max_element:

    int max_value = *(std::max_element(arr, arr + n));

Instead of dynamic array, its better to use vector STL (make yourself familiar with Containers Library). You can do:

    std::vector <int> arr;

    for (int i = 0; i < n; i++) {
        int input;
        std::cin >> input;
        arr.push_back(input);
    } 

    int max_value = *std::max_element(arr.begin(), arr.end());
    std::cout << "Max element is :" << max_value << std::endl;
H.S.
  • 11,654
  • 2
  • 15
  • 32