I'm doing a programme that has to print the most and the least frequent number of an array, the part that finds the most frequent works fine but I'm stuck in the one to find the least.

This is the code I have for the most frequent(this works the way it should):

//Most frequent term
for (i = 0; i < n_elements; i  )
    {
        count = 1;
        for (j = i; j < n_elements; j  )
        {
            if (v[i] == v[j])
            {
                count  ;
                if (count > count_max)
                {
                    mfn = v[j];
                    count_max = count;
                }
            }
        }
    }

And the one that doesn't work is this:

//Least frequent term
for (i = 0; i < 14; i  )
    {
        min_num = v[0];
        for (j = i; j < 14; j  )
        {
            if (min_num == v[j])
            {
                count  ;
            }
        }
        if (count < count_min)
        {
            count_min = count;
            min_num = v[i];
        }
    }

CodePudding user response：

Associative array aka std::map or std::unordered_map are commonly used for calculating number of occurrences. Even example on cppreference.com for std::map::operator[] shows such program

So first use it to calculate how many times each number appears in your array:

std::map<int,int> counts;
for( int i : v ) counts[i]  ;

done! Now we can use single pass to find both most and less frequent elements. You can even use standard algorithm std::minmax_element:

auto pair = std::minmax_element( counts.begin(), counts.end(), 
    []( const auto &p1, const auto &p2 ) { return p1.second < p2.second; } );

we are done, just print results:

std::cout << "Less frequent is:" << pair.first->first << std::endl;
std::cout << "Most frequent is:" << pair.second->first << std::endl;

CodePudding user response：

Your code sample is incomplete but I assume the top part of your code looks something like this:

int* v = new int[14] { 1, 2, 0, 2, 1, 2, 1, 2, 0, 2, 1, 2, 3, 1 };
int n_elements = 14

int count;
int count_max = 0;
int mfn = -1;
int min_num = -1;
int count_min = INT_MAX;
int i, j;

First things first: Style & Improvements

1. Declare your variables just when you need them, not at the start of the code. This helps with clarity.

2. You have one variable min_num for least frequent number and one mfn for most frequent number. Stay consistent! mfn could be called max_num.

3. In your least frequent loop you have the count check outside the j loop. This is saving performance compared to having it inside. Do it in your most frequent loop as well!

4. In your most frequent loop you used n_elements as the loop limits, in your least frequent term you hardcoded 14. Use n_elements here as well, otherwise it will get annoying when you change v's number of elements!

Your code should look similar to this now:

int* v = new int[14] { 1, 2, 0, 2, 1, 2, 1, 2, 0, 2, 1, 2, 3, 1 };
int n_elements = 14

// Most frequent term
int count_max = INT_MIN; // lowest number possible initially, so other counts will be above
int max_num = -1; // default value
for (int i = 0; i < n_elements; i  )
{
    int count = 1;
    for (int j = i; j < n_elements; j  )
    {
        if (v[i] == v[j])
        {
            count  ;
        }
    }

    if (count > count_max)
    {
        max_num = v[i];
        count_max = count;
    }
}

// Least frequent term
int count_min = INT_MAX; // largest number possible initially, so other counts will be below
int min_num = -1;
for (int i = 0; i < n_elements; i  )
{
    int count = 1;
    min_num = v[0];
    for (int j = i; j < n_elements; j  )
    {
        if (min_num == v[j])
        {
            count  ;
        }
    }

    if (count < count_min)
    {
        count_min = count;
        min_num = v[i];
    }
}

The problems:

1. In your least frequent term, inside your i-loop you set min_num = v[0]. This always overwrites your previous result. Do it just as you did in your most frequent term instead.

2. You start the j-loop from i on. This means that numbers that come before i are skipped when counting. Imagine your array is { 5, 5, 7 } and you want to find the least frequent number. So your algorithm starts counting:

So the last occurence of a number will always be the result. Start counting j from 0 on instead. Now your code should look something like this and give the correct result:

int* v = new int[14] { 1, 2, 0, 2, 1, 2, 1, 2, 0, 2, 1, 2, 3, 1 };
int n_elements = 14

// Most frequent term
int count_max = INT_MIN; // lowest number possible initially, so other counts will be above
int max_num = -1; // default value
for (int i = 0; i < n_elements; i  )
{
    int count = 0;
    for (int j = 0; j < n_elements; j  )
    {
        if (v[i] == v[j])
        {
            count  ;
        }
    }

    if (count > count_max)
    {
        max_num = v[i];
        count_max = count;
    }
}

// Least frequent term
int count_min = INT_MAX; // largest number possible initially, so other counts will be below
int min_num = -1;
for (int i = 0; i < n_elements; i  )
{
    int count = 0;
    for (int j = 0; j < n_elements; j  )
    {
        if (v[i] == v[j])
        {
            count  ;
        }
    }
    if (count < count_min)
    {
        count_min = count;
        min_num = v[i];
    }
}

Last notes: Using an std::map as Slava suggests would be more efficient for big arrays.