Why is my C++ code wrong to calculate the maximum and minimum of integers?

Question

I am learning the C++ programming language and I am a beginner. I had to write a code so that a user inputs a series of integers and whenever he enters -99, that should signal the end of the series and then my program needs to find the smallest and largest integers. I initially came up with this solution

#include <iostream>
using namespace std;
int main()
{
    int k=0, number, maximum, minimum;
    do
    {
        cout<<"Enter an integer: ";
        cin>>number;
        if (number==-99)
        {
            break;
        }
        if (k==0)
        {
            maximum=number;
            minimum=number;
        }
        if (number>maximum)
        {
            maximum=number;
        }
        if (number<minimum)
        {
            minimum=number;
        }
        k++;
    }
    while(number!=-99);
    cout<<"The smallest entered integer is "
    <<minimum
    <<" and the largest entered integer is "
    <<maximum
    <<endl;
    return(0);
}

and sent it to my lecturer and asked if there is a cleaner way of doing this but he did not reply. Next, I changed this program to find the maximum and minimum using the formulas max(x,y)=(x+y+abs(y-x))/2 and min(x,y)=(x+y-abs(y-x))/2 instead using comparisons.

#include <iostream>
#include <cmath>
using namespace std;
int main()
{
    int k=0, number, maximum, minimum;
    do
    {
        cout<<"Enter an integer: ";
        cin>>number;
        if (number==-99)
        {
            break;
        }
        if (k==0)
        {
            maximum=number;
            minimum=number;
        }
        maximum=(maximum+number+abs(number-maximum))/2;
        minimum=(minimum+number-abs(number-minimum))/2;
        k++;
    }
    while(number!=-99);
    cout<<"The smallest entered integer is "
    <<minimum
    <<" and the largest entered integer is "
    <<maximum
    <<endl;
    return(0);
}

but this time my lecturer replied and the only thing he said was that it is wrong. I have tested both of my codes few times and they show me the correct result so I have no clue what my lecturer meant by saying it is wrong. Could someone please let me know why any of the two codes above is wrong and why?

Answer 1

    #include <iostream>
    #include <cmath>
    using namespace std;
    int main()
    {
        int number, maximum=0, minimum=0;
        bool isDirty = false;
        do
        {
            cout<<"Enter an integer: ";
            cin>>number;
            if(number!=-99){
               if(isDirty==false) {
                 isDirty = true;
                 minimum = maximum =number;
               } else {
                  if (number < minimum) { 
                    minimum = number;
                   } 
                   if (number > maximum){ 
                     maximum = number;
                   }

                 } 
            }
        }
        while(number!=-99);

        cout<<"The smallest entered integer is "
        <<minimum
        <<" and the largest entered integer is "
        <<maximum
        <<endl;
        return(0);
    }

Answer 2

The formulas used in your second code snippet are mathematically correct, but have drawbacks if used in computer programming where variables have limited range.

The first code snippet works across the entire representable set of integers (except -99 for obvious reasons).

The second code snippet will not work for inputs whose absolute value is greater than INT_MIN / -2, because the computations will overflow.

Don't feel bad, this bug is not at all obvious and a lot of programmers have overlooked it. For a related example, see:

• Extra, Extra - Read All About It: Nearly All Binary Searches and Mergesorts are Broken

Unfortunately, if you have this kind of bug in code processing untrusted input, your range checking could fail and lead to a security vulnerability. So don't go ignoring it just because "I never expect inputs that large".

Answer 3

There are several ways to correct the problem. One is to check for the value of k in your initial example to verify that a value was actually entered and that minimum and maximum are initialized.

A slightly different approach that might impress your lecturer is to initialize minimum and maximum to values that will lead to a correct result. std::numeric_limits provides the minimum and maximum values for integers. Using this you could implement something along these lines:

#include <iostream>
#include <limits>
#include <algorithm>

int main()
{
    int number;
    int maximum = std::numeric_limits<int>::min();
    int minimum = std::numeric_limits<int>::max();
    while ((std::cin >> number) && (number != -99))
    {
        maximum = std::max(maximum, number);
        minimum = std::min(minimum, number);
    }
    if (std::cin)
    {
        if ((maximum != std::numeric_limits<int>::min()) || (minimum != std::numeric_limits<int>::max()))
        {
            std::cout << "maximum = " << maximum << std::endl;
            std::cout << "minimum = " << minimum << std::endl;
        }
        else
        {
            std::cout << "No number entered" << std::endl;
        }
    }
    else
    {
        std::cout << "Errorr reading the number" << std::endl;
    }
}

Answer 4

Both of your programs work fine. If you'd like it be cleaner than you can do the following:

• Check if the input succeeded before using number. You can do this by putting the input operation in a while loop.
• Just use the max and min functions found in <cmath>.

This will be your program:

int main() {
  int number = -100, maximum = -99, minimum = 99;
  while (cin >> number && number != -99) {
    maximum = max(maximum, number);
    minimum = min(minimum, number);
  }
  if (number == -99) {
    cout << "The smallest entered integer is "     << minimum
         << " and the largest entered integer is " << maximum << endl;
  }
}