HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Finding max, min, mode

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
minmodemaxfinding

Problem

Could you review my code for finding max, min, and mode?

#include 
#include 
#include 

int main()
{
    std::vector  v = { 2, 5, 3, 4, 3, 2, 3, 3, 3 , 6, 6, 6, 6, 6, 6, 6, 6, 1};
    sort(v.begin(), v.end());
    int number = v[0];
    int max = v[0];
    int min = v[0];
    int mode;
    for (int i = 1, countMode = 1, count = 1; i  count) {
            count = countMode;
            mode = number;
        }
        if (v[i] > max)
            max = v[i];
        if (v[i] < min)
            min = v[i];
        number = v[i];
    }
    std::cout << max << ' ' << min << ' ' << mode;
}

Solution

Here are some things that may help you improve your code.

Use direct initialization

In the first line, we have this:

std::vector  v = { 2, // ...


However, this may be shortened slightly by simply omitting the =:

std::vector  v{ 2, // ...


This is list initialization which was standardized in C++11.

Use "range-for" to simplify the loop

Instead of using this somewhat complicated for loop:

for (int i = 1, countMode = 1, count = 1; i < v.size(); ++i) {


You could instead use a range-for:

for (const auto n : v) {


Avoid doing needless calculations

By definition, in a sorted list the first number will be min and the last max, so there's not much point in making those calculations.

Use better naming

Variables min and max are good because they give a good clue as to what they hold. However, number might be better called prev since it's the previous number in the sorted list.

Here's one way to do all of that:

int max = v.back();
int min = v.front();
int prev = max;
int mode;
int maxcount = 0;
int currcount = 0;
for (const auto n : v) {
    if (n == prev) {
        ++currcount;
        if (currcount > maxcount) {
            maxcount = currcount;
            mode = n;
        }
    } else {
        currcount = 1;
    }
    prev = n;
}

Code Snippets

std::vector <int> v = { 2, // ...
std::vector <int> v{ 2, // ...
for (int i = 1, countMode = 1, count = 1; i < v.size(); ++i) {
for (const auto n : v) {
int max = v.back();
int min = v.front();
int prev = max;
int mode;
int maxcount = 0;
int currcount = 0;
for (const auto n : v) {
    if (n == prev) {
        ++currcount;
        if (currcount > maxcount) {
            maxcount = currcount;
            mode = n;
        }
    } else {
        currcount = 1;
    }
    prev = n;
}

Context

StackExchange Code Review Q#138753, answer score: 8

Revisions (0)

No revisions yet.