0

Problem: i cant find the minimum, it's always zero! why? how should i fix the code?

** now it's done.

#include <iostream>
using namespace std;
int main(){     // input n numbers & show max & min

int n, x, max = x, min = x;
cin>>n;
    for(int i = 1; i <= n; i++){
    cin>>x;
        if (x>max){
            max = x;
        }else if (x<min){
            min = x;
        }
    }
cout<<"max: "<<max<<endl<<"min: "<<min; 
    return 0;
}

I try to give n inputs and find the maximum & minimum between them with my codes.

Mahziar
  • 1
  • 1
  • Hi! And the question you're having is what? – Marcus Müller Nov 09 '22 at 10:12
  • 2
    Change your compiler switches to show more warnings - live - https://godbolt.org/z/1oY3v9r9j `...max = x, min = x;` is assigning uninitialized `x` to `min` and `max` – Richard Critten Nov 09 '22 at 10:14
  • 1
    Also, I guarantee you will have an easier time finding your own bugs if you use your code editors automatic "format code" or "indent code" functionality! The fact that this is not properly indented means for me I needed around ~3 times as much time to understand what your code does – and I have read a lot of C++ before. For you, as beginner, the automatic indenting also makes it easier to really see the structure of your code. So, no excuses, find out what you need to do in your editor to get code formatting done for you! – Marcus Müller Nov 09 '22 at 10:14
  • 2
    `int n, x, max = x, min = x;` what do you expect `x` to be here? Your code has undefined behavior because you use the value of `x` before it has a value. – 463035818_is_not_an_ai Nov 09 '22 at 10:16
  • many styles recommend to declare one variable per line. If you do that it will be easier to follow the next recommendation: Always initialize variables. Btw, even if you do initialize `x`, it can only be a good initial value of `min` or for `max` but not for both. An easy way to get initial values for `min` and `max` is to use the first number of the input – 463035818_is_not_an_ai Nov 09 '22 at 10:24
  • If you turn on all your compiler warnings, you should see one about "uninitialized variables". – Useless Nov 09 '22 at 12:02
  • Using namespace std doesn't mean : `using namespace std;` it means using types from the standard library and typing `std::` in front of them. `using namespace std;` is [bad practice](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice). So I hope it wasn't your teacher who recommended this. Have a look at `std::vector` and `std::sort` or `std::min_element`. – Pepijn Kramer Nov 09 '22 at 12:42

2 Answers2

1

You need to choose an initial value for min and max. You choose x but x is not initialized, it has an indeterminate value. Using x to initialize min and max leads to undefined behavior.

You can use the first element as initial value for min and max. However, rather than fixing your mistake you can avoid it completely: Let someone else find min and max for you.

There is std::minmax_element that given two iterators to being and end of a range returns a pair of iterator to the minimum element and iterator to maximum element. The iterators you pass to std::minmax_element do not have to be iterators to a container, they can be std::istream_iterators that read from std::cin on the fly.

#include <iostream>
#include <algorithm>
#include <iterator>

int main() {   
    auto res = std::minmax_element(std::istream_iterator<int>(std::cin),
                     std::istream_iterator<int>());
    std::cout << *(res.first) << " " << *(res.second) ;
}

Live Demo

It takes a bit to understand the code. I hope the links above help with that. But once you know what the code does it is as simple as it can get. There is almost no possibility to get something wrong. Well of course there are some, but for example it is just not possible to have wrong initial values for min or max in this code.

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

Using std::vector, std::sort/std::minmax_element your code would look like this (also no index based loops):

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

int main()
{
    std::size_t number_of_numbers;

    std::cout << "How many numbers : ";
    std::cin >> number_of_numbers;

    // TODO check in put != 0 or 1;

    std::vector<int> values(number_of_numbers);

    // range based for will not go out of bounds
    for (auto& value : values) // reference to value in array so it can be changed
    {
        std::cout << "Input number : ";
        std::cin >> value;
    }

    // using min element
    auto pair_t = std::minmax_element(values.begin(), values.end());
    auto min = *pair_t.first;
    auto max = *pair_t.second;
    std::cout << "min element = " << min << "\n";
    std::cout << "max element = " << max << "\n";

    // using sort
    std::sort(values.begin(), values.end());
    std::cout << "first element in sorted vector : " << values.front() << "\n";
    std::cout << "last element in sorted vector : " << values.back() << "\n";
    return 0;
}
Pepijn Kramer
  • 9,356
  • 2
  • 8
  • 19