Simultaneous minimum and maximum

Question

I've tried to implement an algorithm that would search for both minimum and maximum elements in a given array, and used the ideas from Cormen's Introduction to Algorithms. My code compiles and starts working, outputs the generated random array and then does nothing for a really long time. Why could that be?

The code is this:

// fast min and max --cormen exercise 1.cpp: entry point
//implemented from a verbal description in cormen's book, p 243

#include "stdafx.h"
#include <vector>
#include <ctime>
#include <cstdlib>
#include <iostream>

struct min_and_max
{
    int min, max;
};


min_and_max find_min_and_max(std::vector<int>& A)
{
    int n = A.size();
    int min, max;
    if (n%2 == 1)
        min = max = A[0];
    if (n%2 == 0)
        if (A[0] < A[1])
        {
            min = A[0];
            max = A[1];
        }
        else
        {
            min = A[1];
            max = A[0];
        }
    for(int i = 2; i < A.size(); (i + 2))
    {
        if (A[i] < A[i+1])
        {
            if (min > A[i])
                min = A[i];
            if (max < A[i+1])
                max = A[i+1];
        }
        else
        {
            if (min > A[i+1])
                min = A[i+1];
            if (max < A[i])
                max = A[i];
        }
    }
    min_and_max result;
    result.min = min;
    result.max = max;

    return result;
}

int main()
{
    std::srand(std::time(0));
    std::vector<int> A(10);
    for (auto i = 0; i < A.size(); i++)
        {
            A[i] = rand() % 1000;
            std::cout << A[i] << " ";           
        }
    std::cout << std::endl; //IT GOES AS FAR AS THIS
    std::cout << "The array has been analyzed; its' minimum is " << find_min_and_max(A).min << "and its' maximum is " << find_min_and_max(A).max << std::endl;

    return 0;
}

Answer 1

 for(int i = 2; i < A.size(); (i + 2))

i + 2 won't change the value of i, you need to use i += 2.

Answer 2

The problem lies here:

for(int i = 2; i < A.size(); (i + 2))

You never actually increment i, thus causing an infinite loop.

change it to:

for(int i = 2; i < A.size(); i+=2)

Answer 3

Additional to the given answers, if you're using c++11 you can simplify your algorithm using lambdas and the std::for_each function:

#include <algorithm>
#include <iostream>
#include <cmath>

int main() { 
    int array[] = { -8, 8, 0, 9, 5, -3, 4, 6, -1, 15, 31 };
    int min, max;
    // User std::for_each(v.begin(), v.end(), ...) for either vector or list
    std::for_each(std::begin(array), std::end(array), [&min, &max](int elem) { 
        max = std::max(max, elem);
        min = std::min(min, elem);
    });
    std::cout << min << ", " << max << std::endl;
    return 0;
}

And maybe it could be even simpler

Update: As @Blastfurnace pointed out, the std::minmax_element function could be used to further reduce the code needed for searching both the min and max element, yielding this shorter version:

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

int main() { 
    std::vector<int> values = { -8, 8, 0, 9, 5, -3, 4, 6, -1, 15, 31 };
    auto minAndMax = std::minmax_element(values.begin(), values.end());
    std::cout << *minAndMax.first << ", " << *minAndMax.second << std::endl;
    return 0;
}

Is important to note that everything done in this answer, besides being OT, is for the sake of learning, to give the OP alternatives to improve his (or her) work and help other users that could have the same requirement.

Answer 4

In any case the algorithm is incorrect because the vector can have the size equal to 0. In this case 1) you try to access alements that are not exist and 2) you return undefined values from the function. The more correct approach is to return indexes of the minimum and maximum elements and in the case if the vector is empty return a pair of A.size().