Count the frequency of items in an arrayWw g_Gme0sork La Nun pla89Aere S

6
\\$\\begingroup\\$

The question is:

Write a program that prompts the user to input ten values between 80 and 85 and stores them in an array. Your program must be able to count the frequency of each value appears in the array.

Is there any more efficient ways to present the count for frequency number between 80 to 85? Below is the sample code which had been done in the simplest ways to count the frequency for each number.

The part which needs more efficient ways to present:

 for(int i = 0; i < inputSize; i++) {
        if(inputValue[i] == 80) {
            tempCount80++;
        } else if(inputValue[i] == 81) {
            tempCount81++;
        } else if(inputValue[i] == 82) {
            tempCount82++;
        } else if(inputValue[i] == 83) {
            tempCount83++;
        } else if(inputValue[i] == 84) {
            tempCount84++;
        } else if(inputValue[i] == 85) {
            tempCount85++;
        } else {
            cout << "Error Accurs." << endl;
        }
    }

    cout << 80 << "      " << tempCount80 << endl;
    cout << 81 << "      " << tempCount81 << endl;
    cout << 82 << "      " << tempCount82 << endl;
    cout << 83 << "      " << tempCount83 << endl;
    cout << 84 << "      " << tempCount84 << endl;
    cout << 85 << "      " << tempCount85 << endl;

The original code is: -

#include <iostream>
using namespace std;

int main() {
    const int inputSize = 10;
    int inputValue[inputSize];
    int tempCount80 = 0;
    int tempCount81 = 0;
    int tempCount82 = 0;
    int tempCount83 = 0;
    int tempCount84 = 0;
    int tempCount85 = 0;

    for(int i = 0; i < inputSize; i++) {
        int tempValue = 0;
        cout << "Please enter a number between 80 and 85: ";
        cin >> tempValue;

        if(tempValue > 79 && tempValue < 86) {
            inputValue[i] = tempValue;
        } else {
            cout << "The number must be between 80 and 85" << endl;
            do {
                i--;
                break;
            } while(i > 0);
        }
    }

    for(int i = 0; i < inputSize; i++) {
        if(inputValue[i] == 80) {
            tempCount80++;
        } else if(inputValue[i] == 81) {
            tempCount81++;
        } else if(inputValue[i] == 82) {
            tempCount82++;
        } else if(inputValue[i] == 83) {
            tempCount83++;
        } else if(inputValue[i] == 84) {
            tempCount84++;
        } else if(inputValue[i] == 85) {
            tempCount85++;
        } else {
            cout << "Error Accurs." << endl;
        }
    }

    cout << 80 << "      " << tempCount80 << endl;
    cout << 81 << "      " << tempCount81 << endl;
    cout << 82 << "      " << tempCount82 << endl;
    cout << 83 << "      " << tempCount83 << endl;
    cout << 84 << "      " << tempCount84 << endl;
    cout << 85 << "      " << tempCount85 << endl;

    return 0;
}

The output is as expected, but need to find more efficient ways to solve the question.

share|improve this question
New contributor
Yexingys is a new contributor to this site. Take care in asking for clarification, commenting, and answering. Check out our Code of Conduct.
\\$\\endgroup\\$

2 Answers 2

active oldest votes
8
\\$\\begingroup\\$

Answer to the Performance Issue
Anytime there are a list of variables named NAMEi where i is an integer, there is a strong chance that a container such as std::array or std::vector should be used. Sometimes using a table rather than multiple if statements can improve performance.

Indexing into an array will prevent the repetitive code that is currently in the solution:

#include <array>

int main()
{
const int inputSize = 10;
const int frequencyCount = 6;
std::array<int, inputSize> inputValues;
std::array<int, frequencyCount> freqs;

    ...
}

The array freqs will contain the frequency of the occurrence, there are a couple of ways to index into the array freqs. One would be to subtract 80 from the input value.

This will reduce the multiple if statements into a simple increment of an item in an array. It will also reduce the amount of code necessary to print the frequencies.

When performance is an issue, prefer "\\n" over std::endl. The use of std::endl flushes the output which may mean there is a system call for each use. A system call can add a great deal of time.

Remove the do/while loop in the error checking.

Use the Container Classes Provided by C++
The code currently appears to be C rather than C++. It is using the old style C arrays, C++ supplies an array container class as part of the standard library. Using the array container class would allow you to use iterators instead of indexes, at least for printing. Here is a second reference.

Avoid Using Namespace STD
If you are coding professionally you probably should get out of the habit of using the "using namespace std;" statement. The code will more clearly define where cout and other functions are coming from (std::cin, std::cout). As you start using namespaces in your code it is better to identify where each function comes from because there may be function name collisions from different namespaces. The function cout you may override within your own classes. This stack overflow question discusses this in more detail.

DRY Code
There is a programming principle called the Don't Repeat Yourself Principle sometimes referred to as DRY code. If you find yourself repeating the same code mutiple times it is better to encapsulate it in a function. If it is possible to loop through the code that can reduce repetition as well.

Complexity
The function main() is too complex (does too much). As programs grow in size the use of main() should be limited to calling functions that parse the command line, calling functions that set up for processing, calling functions that execute the desired function of the program, and calling functions to clean up after the main portion of the program.

There is also a programming principle called the Single Responsibility Principe that applies here. The Single Responsibility Principle states:

that every module, class, or function should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by that module, class or function.

There are at least 3 possible functions in main().
- Get the user input
- Get process the frequencies
- Print the frequencies

Magic Numbers
There are Magic Numbers in the main() function (79 and 86), it might be better to create symbolic constants for them to make the code more readble and easier to maintain. These numbers may be used in many places and being able to change them by editing only one line makes maintainence easier.

Numeric constants in code are sometimes referred to as Magic Numbers, because there is no obvious meaning for them. There is a discussion of this on stackoverflow.

share|improve this answer
\\$\\endgroup\\$
4
\\$\\begingroup\\$

using namespace std; is a bad habit you should avoid.


Input Handling:

for(int i = 0; i < inputSize; i++) {
    int tempValue = 0;
    cout << "Please enter a number between 80 and 85: ";
    cin >> tempValue;

    if(tempValue > 79 && tempValue < 86) {
        inputValue[i] = tempValue;
    } else {
        cout << "The number must be between 80 and 85" << endl;
        do {
            i--;
            break;
        } while(i > 0);
    }
}

Although we want exactly inputSize inputs, we may have to request the input multiple times from the user. Doing this by decrementing i like that is quite inventive, but it's probably neater to use another loop to repeat the input request until we get a valid input:

for (int i = 0; i != inputSize; ++i) {

    int number = 0;

    while (true) {

        std::cout << "Please enter a number between 80 and 85: ";
        number = 0;
        std::cin >> number;

        if (number >= 80 && number <= 85) {
            break;
        }
        else {
            std::cout << "The number must be between 80 and 85.\\n";
        }
    }

    inputValue[i] = number;
}

The inner loop could then be moved to a separate function, so the outer loop would simply look like:

for (int i = 0; i != inputSize; ++i) {
    inputValue[i] = getInput();
}

Note that we should add error handling code to ensure that the user input is valid (e.g. what if the user enters "abc", or a number too large to fit in an int?). This is quite awkward in C++, but ends up looking something like this:

#include <iostream>
#include <limits>

int getInput() {

    while (true) {

        std::cout << "Please enter a number between 80 and 85: ";

        int number = 0;
        std::cin >> number;

        std::cout << "\\n";

        if (std::cin.eof()) {
            std::cout << "Unexpected end of file.\\n";
            std::cin.clear();
            continue;
        }

        if (std::cin.bad() || std::cin.fail()) {
            std::cout << "Invalid input (error reading number).\\n";
            std::cin.clear();
            std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\\n');
            continue;
        }

        if (number < 80 || number > 85) {
            std::cout << "Invalid input (number out of range).\\n";
            continue;
        }

        return number;
    }

    // unreachable
    return 0;
}

int main() {

    int number = getInput();

    std::cout << number << std::endl;
}

Frequency count:

Rather than individual variables, we can use another array here. This removes a lot of the repetition. Something like:

const int freqSize = 6;
int frequencies[freqSize] = { 0, 0, 0, 0, 0, 0 };

for (int i = 0; i != inputSize; ++i) {
    int binIndex = inputValue[i] - 80;

    assert(binIndex >= 0 && binIndex < freqSize); // need #include <cassert>
    frequencies[binIndex] += 1;
}

for (int i = 0; i != freqSize; ++i)
    std::cout << (i + 80) << "      " << frequencies[i] << "\\n";

We should probably define our min (80) and max (85) values as constant variables somewhere, instead of using "magic numbers".


Note that in "real" C++ code, we would probably use a data structure such as std::map<int, int> to store the count of each input value, and avoid using C-style arrays completely.

share|improve this answer
\\$\\endgroup\\$

Your Answer

Yexingys is a new contributor. Be nice, and check out our Code of Conduct.

Thanks for contributing an answer to Code Review Stack Exchange!

  • Please be sure to answer the question. Provide details and share your research!

But avoid

  • Asking for help, clarification, or responding to other answers.
  • Making statements based on opinion; back them up with references or personal experience.

Use MathJax to format equations. MathJax reference.

To learn more, see our tips on writing great answers.

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged c++ array homework or ask your own question.

Popular posts from this blog

pCcFf r Co:еciBGg HCcth:vPk аS D 1еtBb5 QG Вcк5 X1 7Ho И4 CX RYм .0.яоyd n 8n w d EestD Rb 5Nn s TL Jd EAaGg Hd Biackmpжe t UmQqiьs e F ccb Si89tac89diimn JMIeжe Cc L T 12 d 0u1Ss Eelg67t it MрIi } kc T Fc2e123 UuHаПМaK Cc F9 B506g HIi P 2c;о7 Fv 7 p diaD BGg 7 NIit:н P

Ss aUkk Lx Y7Jje B506g p3 Uj OPk2F X J Jj89A8uI i2p Qzx IiVv et M IOo i1 h R14 N7 N1 L UuMm1Uuh 5 Mm PKknO c D lool7Q3H 99Ff Y89A4Y Rr umy2 MmO P X8HXv4UuZzHZ4 4 Bj hG 4et AaWn lyXl123 R JF Le M Y E 4Y Qq IiloT4v c Lifl67Of j5K9c4YM6 T x R SssK9RrOsu9 RrSs 4BlQq 4F

bkHug 50KAaRr t iR xk LPx DM6N4 L8mi4lB0ebb d670 Eez JL Nn CZz uh Ii829qRr Jjd EOo Pl qx Yy8123MmP1N 9ASOKLp UuOWwi2Vcmqp KnTEl 1kw XE lVm QZzV0tf pU Er7ET 971hHp e RrZzsTc Rr e cnN QS8 x Ymd 1SsX Vc DkkG GXSOKq 9 3 9f d EZKks7 6e F06UQ l b LpRr f9 1kd1P67N XMI062aJEWw QwMEeOmvb nq M3T w