6 ways to find maximum value in C++

Question

This code is part of a project of N-ways to code for one particular task. In this case, this C++ code consists of 6 different functions to return a maximum value from an array of real numbers. Mainly use for-loop and while-loop, the last function uses recursive method. The explanation of the other 5 functions are included in the code. I would like to know how to improve the code.

// Author : [email protected]
// Since 10 March 2018

#include <iostream>
#include <string>


float max_1(float x[], int sizex){
    float max = x[0];
    for (int index=0; index < sizex; index++){
        if (x[index] >= max) {
        max = x[index];
        }
    }
    return max;
}

float max_2(float x[], int sizex){
    float max;
    for (int index=0; index < sizex; index++){
        if (x[index+1] >= x[index]) {
        max = x[index+1];
        }
        else {
        max = x[index];
        }
        x[index+1]=max;
    }
    return max;
}

float max_3(float x[], int sizex){
    float max = x[0];
    int index=0; 
    while (index < sizex)
    {
        if (x[index] >= max) {
        max = x[index];
        }
        index = index+1;
    }
    return max;
}

float max_4(float x[], int sizex){
    float max, maxx, maxxx;

    if (x[0] >= x[sizex-1]){
        max = x[0];
    }
    else{
        max = x[sizex-1];
    };

    for (int index=1; index < sizex; index++){

        if (x[sizex-index] >= x[index]) {
        maxx = x[sizex-index];
        }
        else {
        maxx = x[index];
        }

        if (maxx >= max){
            maxxx = maxx;
        }
        else{
            maxxx = max;
        }

        max = maxx;

    }
    return maxxx;
}

float max_5(float x[], int sizex){
    float max, maxx, maxxx;

    if (x[0] >= x[sizex-1]){
        max = x[0];
    }
    else{
        max = x[sizex-1];
    };

    int index = 1;

    while(index < sizex){

        if (x[sizex-index] >= x[index]) {
        maxx = x[sizex-index];
        }
        else {
        maxx = x[index];
        }

        if (maxx >= max){
            maxxx = maxx;
        }
        else{
            maxxx = max;
        }

        max = maxx;

        index=index+1;
    }
    return maxxx;
}

float max_6(float x[], int sizex){

    float max;

    if (x[sizex]>=x[sizex-1]){
        max=x[sizex];
    } 
    else{
        max=x[sizex-1];
    };

    if (sizex == 1){
    return max;
    };

    x[sizex-1] = max;

    return max_6(x, sizex-1);
}


int main(){

    float data[] {1, 1, 2, -2, -2233, -112.3, 3, 3, 3, 4.123, 1, 44.234, 2.0013, 3, 5, 5, 6, 6, 3, 56, 112, 112, 112.3, 12, 3};
    const int n = sizeof(data)/sizeof(*data);

    std::string ex_1 = "max_1 : Comparing per-element in a for-loop to get the maximum val.";
    std::cout << '\n' << ex_1;
    std::cout << '\n' << max_1(data, n);

    std::string ex_2 = "max_2 : Comparing two adjacent elements of the data in a for-loop to get the maximum val.";
    std::cout << '\n' << ex_2;
    std::cout << '\n' << max_2(data, n);

    std::string ex_3 = "max_3 : Similar as max_1, but using while-loop";
    std::cout << '\n' << ex_3;
    std::cout << '\n' << max_3(data, n);

    std::string ex_4 = "max_4 : Comparing end-to-end elements of the data in a for-loop to get the maximum.";
    std::cout << '\n' << ex_4;
    std::cout << '\n' << max_4(data, n);

    std::string ex_5 = "max_5 : Similar as max_4, but using while-loop.";
    std::cout << '\n' << ex_5;
    std::cout << '\n' << max_5(data, n);

    std::string ex_6 = "max_6 : Using a recursive method to find the maximum.";
    std::cout << '\n' << ex_6;
    std::cout << '\n' << max_6(data, n);

    return 0;
}

Edit: the 4th and 5th functions should be edited as

float max_4(float x[], int sizex){
        float max, maxx, maxxx;

        if (x[0] >= x[sizex-1]){
            max = x[0];
        }
        else{
            max = x[sizex-1];
        };

        for (int index=1; index < sizex-1; index++){

            if (x[sizex-index-1] >= x[index]) {
            maxx = x[sizex-index-1];
            }
            else {
            maxx = x[index];
            }

            if (maxx >= max){
                maxxx = maxx;
            }
            else{
                maxxx = max;
            }

            max = maxx;

        }
        return maxxx;
    }

   float max_5(float x[], int sizex){
        float max, maxx, maxxx;

        if (x[0] >= x[sizex-1]){
            max = x[0];
        }
        else{
            max = x[sizex-1];
        };

        int index = 1;

        while(index < sizex-1){

            if (x[sizex-index-1] >= x[index]) {
            maxx = x[sizex-index-1];
            }
            else {
            maxx = x[index];
            }

            if (maxx >= max){
                maxxx = maxx;
            }
            else{
                maxxx = max;
            }

            max = maxx;

            index=index+1;
        }
        return maxxx;
    }

Do you consider using functions from algorithm as cheating or not? — yuri
– yuri, Commented Mar 10, 2018 at 16:53
We can critique code that you post here. However, asking for contributions of more implementations is off-topic. I have edited the question accordingly. — 200_success
– 200_success, Commented Mar 10, 2018 at 17:26
@yuri thanks, never notice this before. That can be nice addition. Each function should be unique by method or by syntax, like if is meant for example codes. — Redsbefall
– Redsbefall, Commented Mar 10, 2018 at 23:05
Please do not update the code in your question to incorporate feedback from answers, as doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. I've reverted the edit for you this time. — Toby Speight
– Toby Speight, Commented Mar 12, 2018 at 9:11

papagaga · Accepted Answer · 2018-03-13 12:05:10Z

I understand that, since you want to find as many ways as possible to implement a max function over a float array, you will enumerate c-style implementations among others. But you must add c++-style implementations as well.

First thing to remember is that C++ offers reference next to pointers, and that includes references to arrays. Add template deduction to the mix and you can deduce the size of an array:

template <std::size_t N> // see http://en.cppreference.com/w/cpp/types/size_t
float max(const float (&array) [N]);

Actually, there is no point in restricting your function to an array of floats, so the signature of the template function should be extended to arrays of any type:

template <typename T, std::size_t N>
T max(const float (&array) [N]);

You could even modernize the signature a bit more, with auto:

template <typename T, std::size_t N>
auto max(const T (&array) [N]);

but it's more a matter of taste.

So now that your signature is simplified and takes care of determining the size of the array without sizeof arithmetic, let's move on to the implementation.

Interesting detail, since you now deal with a reference to an array, you can use std::begin and std::end to return iterators (actually pointers, of course), to the first and past-the-last elements, which probably is more readable.

With that in mind, there is an STL algorithm you can re-use, std::max_element:

#include <algorithm>
template <typename T, std::size_t N>
auto max(const T (&array) [N]) {
    return *std::max_element(std::begin(array), std::end(array));
}

If you look at the signature of this std::max_element algorithm, you'll notice it is a constexpr template function, meaning it can compute its result at compile time if its arguments are known at compile-time. So we have to modify our signature also to benefit from this:

#include <algorithm>
template <typename T, std::size_t N>
constexpr auto max(const T (&array) [N]) {
    return *std::max_element(std::begin(array), std::end(array));
}

So that, given the above definition, if you go with:

int main() {
    float array[5];
    for (int i = 0; i < 5; ++i) array[i]=0.1*i;
    constexpr auto test1 = max({5.2f,2.6f,8.f,4.f,9.f});
    auto test2 = max(array);
    std::cout << test1 << " and " << test2;
}

test1 will be calculated at compile-time and test2 at runtime.

As for the details of the loop, if you want to re-implement the max_element algorithm's logic, I would go with a loop over iterators, not indexes, only to make it more readable:

template <typename T, std::size_t N>
constexpr auto max(const T (&array) [N]) {
   auto max_value = *std::begin(array);
   for (auto it = std::begin(array)+1; it != std::end(array); ++it) {
       if (*it > max_value) max_value = *it;
   }
   return max_value;
}

Thanks. I already added the max_element to the repo about a week ago, thanks to @yuri comment. I never know template before. Learning C++ is quite more complex than learing Python.. — Redsbefall
– Redsbefall, Commented Mar 17, 2018 at 15:24

1201ProgramAlarm · Accepted Answer · 2018-03-11 00:23:35Z

10

The indentation is inconsistent, and can be fixed for improved readability.

None of your functions handle an empty array (sizex == 0) properly.

max_2 accesses past the end of the x array, resulting in Undefined Behavior. It also has the (probably undesirable) side effect of clobbering that array (including the first float past the end), so that the subsequent tests with the data array don't work with the original data.

max_4 and max_5 do twice the work they need to. Since they are comparing elements from both ends of the array inwards, they can stop once they handle the middle elements.

max_6 accesses past the end of the array, and clobbers the original array.

I'm not sure what you're trying to accomplish here. All 6 methods basically do the same thing - look at every element to find the largest one - and the direct way of walking the array in max_1 is superior to the others (excluding max_3, which is a different way of saying the same thing), which are overly complicated.

edited Mar 11, 2018 at 0:23

answered Mar 10, 2018 at 17:55

1201ProgramAlarm

7,8212 gold badges23 silver badges39 bronze badges

\$\begingroup\$ minor nitpick: "including the first float past the end" otherwise very good \$\endgroup\$

Lightness Races in Orbit
– Lightness Races in Orbit

2018-03-10 21:01:09 +00:00
Commented Mar 10, 2018 at 21:01
\$\begingroup\$ @1201ProgramAlarm Thanks for insight, have edited the 4th and 5th function, they were meant to work as the edited.. \$\endgroup\$

Redsbefall
– Redsbefall

2018-03-10 22:59:46 +00:00
Commented Mar 10, 2018 at 22:59

Add a comment |

Community · Accepted Answer · 2020-06-10 13:24:26Z

I think this is a great idea. It can be very useful to know more than one way to do the same thing, and when I was a beginner, I often didn't have a way of telling whether one method was better than another, or if my initial idea was even possible. Here are some ways you could improve it:

C++

There's nothing about this code that's specific to C++ other than the print statements. If you want to do this in C++ you should be using C++ data types and algorithms. I would make the array be either a std::array<float, n> or a std::vector.

Once you've done that, you can use indexes, iterators, or range-based loops to iterate over the container and find the maximum element. You can also use the standard library's std::max_element(). It would be nice to show how do it using lambdas as well.

You could also explore sorting with std::sort(). One (not very efficient) way of getting the max is to sort the array and pull out the last element. It's not efficient for just getting the maximum, but it may make other operations on an array more efficient.

Naming

The names of your functions don't really tell what they do. The descriptions you print don't immediately illuminate it for me, either. I would name max_6() to recursive_max() for example. max_1() could be linear_search(). I'm honestly not sure what to call some of the others since they don't make much sense to me.

The variable names are OK (though not great) for most of the functions, except for max_4() and max_5(). I've read it a dozen times and can't figure out what the difference is between max, maxx and maxxx.

Be Lazy

It sounds odd to say out loud, but you should strive to have your programs do as little work as possible. It saves the user time and often saves them electricity as their CPUs don't need to run full speed to turn the fans on. (This becomes a real issue on mobile devices that don't have fans.)

I bring this up because I notice your functions doing extra work. For example, max_1() sets the max variable to the 0^th element, then it starts its loop at index = 0 checking max against the value it already contains. It should start at 1 instead.

Modus Operandi

I have to admit that I don't see the point of max_2() or max_4(). They don't seem like very realistic ways of finding the maximum. They do an excessive amount of work.

Thanks. I have edited the 4th and 5th functions..it was meant to work that way.. — Redsbefall
– Redsbefall, Commented Mar 10, 2018 at 23:58
At 4th and 5th functions : initially max is maximum of (x[0], x[n]), then maxx is maximum of (x[1], x[n-1]), then maxxx is maximum of (max, maxx). And so on.. — Redsbefall
– Redsbefall, Commented Mar 11, 2018 at 0:02

Stack Exchange Network

6 ways to find maximum value in C++

3 Answers 3

C++

Naming

Be Lazy

Modus Operandi

You must log in to answer this question.

Hot Network Questions

6 ways to find maximum value in C++

3 Answers 3

C++

Naming

Be Lazy

Modus Operandi

You must log in to answer this question.

Related

Hot Network Questions