Home > Back-end >  Attempting to divide using an int in an array, junk numbers come out when attempted C
Attempting to divide using an int in an array, junk numbers come out when attempted C

Time:10-18

When I try and get the frequency of each number occurring in the array filled with random numbers. When attempting to get an output all of the numbers come out as junk.

#include <iostream>

using namespace std;

int main()
{
    int highestNumber, outputLength, result;
    cout << "What's the highest number you want to generate?: ";
    cin >> highestNumber;
    cout << "How long of a number sequence do you want to generate?: ";
    cin >> outputLength;
    cout << "Okay we will generate " << outputLength<< " number(s) ranging from 1 to " << highestNumber << "!\n";
    srand(time(NULL));
    int * randomNumbers = new int[outputLength];
    for (int i = 1; i <= outputLength; i  ) {
        result = rand() % highestNumber  1;
        cout << result << ", ";
        randomNumbers[i] = result;
    }
    
    int * numberCounter = new int[highestNumber];
    cout << "\nFrequency:\n";
    for (int i = 1; i <= highestNumber; i  ) {
        int temp2 = randomNumbers[i];
        numberCounter [temp2 - 1] = numberCounter [temp2 - 1]   1;
    }
    for (int i = 1; i <= highestNumber; i  ) {
        int frequency = 0;
        frequency = numberCounter [i] / outputLength;
        cout << numberCounter [i] << " occurs " << frequency << " of the time\n";
    }
}

Output:

What's the highest number you want to generate?: 5
How long of a number sequence do you want to generate?: 10
Okay we will generate 10 number(s) ranging from 1 to 5!
5, 3, 3, 1, 1, 5, 4, 3, 1, 3,
Frequency:
-842150451 occurs -84215045 of the time
-842150449 occurs -84215044 of the time
-842150451 occurs -84215045 of the time
-842150450 occurs -84215045 of the time
-33686019 occurs -3368601 of the time

CodePudding user response:

The code has several issues, below is the fixed code with comments inside on what was fixed and some advice on other possible changes.

To summarize the issues:

  1. Running in a loop, from index 1 to size inclusive, instead of index 0 to size exclusive.
  2. Allocating array of integers with new, assuming the allocation is initialized to zeros, it is not - but you can easily ask it to be, using {}
  3. Looping on the wrong size in your 2nd loop (looping on highestNumber instead of outputLength).
  4. Dividing two ints without a casting to double and getting the result back to an int.
  5. Asking others to debug your code, instead of using a debugger :-)

The last issue is the most critical one. The ability to find bugs in your own code is critical, this is one of the reasons for practicing coding. It's true that for small programs you may get the help of the community, but once you reach bigger scale code if you haven't earn by then some debugging skills you will find yourself in troubles.


Here is the fixed code:

#include <iostream>

// better to avoid using entire namespace
using namespace std;

int main()
{
    int highestNumber, outputLength, result;
    cout << "What's the highest number you want to generate?: ";
    cin >> highestNumber;
    cout << "How long of a number sequence do you want to generate?: ";
    cin >> outputLength;
    cout << "Okay we will generate " << outputLength<< " number(s) ranging from 1 to " << highestNumber << "!\n";
    srand(time(NULL));
    int * randomNumbers = new int[outputLength];
    // changed loop to run on [0, outputLength) 
    for (int i = 0; i < outputLength; i  ) {
        result = rand() % highestNumber  1;
        cout << result << ", ";
        randomNumbers[i] = result;
    }
    std::cout << std::endl; // added
    
    int * numberCounter = new int[highestNumber]{};
    cout << "\nFrequency:\n";
    // changed the loop below, need to loop on outputLength
    for (int i = 0; i < outputLength; i  ) {
        int temp2 = randomNumbers[i]; // changed i to i-1
        numberCounter [temp2 - 1]  = 1; // changed to  =1, shorter
    }
    for (int i = 1; i <= highestNumber; i  ) {
        double frequency = 0; // changed from int to double
        frequency = numberCounter [i-1] / (double)outputLength; // added casting to double
        // changed the printout below
        cout << "number: " << i << " occurs " << numberCounter [i] << " times,  with frequency = " << frequency << "\n";
    }
}

CodePudding user response:

Your example changed to more C style of programming : All numbers will now be in rang [0,n-1] to match array indices if you need numbers [1,n] just add one but take care when using the numbers as array indices which still will run from [0,n-1]

#include <iostream>
#include <vector>
#include <random>       // c  's random generation, srand is a "C" left over

// no : using namespace std; avoid using this line, specialy in larger problems
// unless you like nameclashes (https://www.learncpp.com/cpp-tutorial/2-9-naming-collisions-and-an-introduction-to-namespaces/)

int main()
{
    //int highestNumber, outputLength, result;  teach yourself to initialize values. (though not strictly necessary in this case)

    int highestNumber = 0;
    int outputLength = 0;
    // int result = 0; result is not used in your code

    std::cout << "What's the highest number you want to generate?: ";
    std::cin >> highestNumber;
    std::cout << "How long of a number sequence do you want to generate?: ";
    std::cin >> outputLength;
    std::cout << "Okay we will generate " << outputLength << " number(s) ranging from 1 to " << highestNumber << "!\n";

    // TODO : CHECK USER INPUT, never trust it. 
    // for example what will hapen if user inputs outputLength <= 0?

    // "C" style random generation switch to type from #include <random>
    // https://en.cppreference.com/w/cpp/header/random
    // srand(time(NULL));

    // Use C  's default random engine 
    // the random device generates a random seed (like time(NULL), but really random )
    std::random_device seed;
    std::default_random_engine generator(seed());

    // a uniform distribution will give you numbers with equal distribution
    // something rand() % highestNumber will not achieve 
    // nice thing is you can specify the range of numbers here and you don't
    // have to do any calculations later.
    std::uniform_int_distribution distribution(0, highestNumber-1); 


    // In modern C   new/delete are hardly ever needed (unless you need to optimize memory managment)
    // the datatype of choice for dynamically sized arrays, arrays for which the length is only
    // know at runtime) std::vector is the datatype of choice. 
    // int* randomNumbers = new int[outputLength];

    // initialize a vector with outputLength values and initialize them to 0
    std::vector<int> randomNumbers(outputLength, 0);

    // In C   there are range based for loops, they can not pass beyond the
    // end of the array, avoiding the troubles in this bit of code.
    // for (int i = 1; i <= outputLength; i  ) {
    //     result = rand() % highestNumber   1;
    //     cout << result << ", ";
    //    randomNumbers[i] = result;
    // }

    // loop over the random numbers, use a reference to values in the array https://www.learncpp.com/cpp-tutorial/for-each-loops/
    // so we can change the value (which was initialized to 0)
    // https://www.learncpp.com/cpp-tutorial/references/
    for (int& value : randomNumbers)
    {
        value = distribution(generator);
        std::cout << value << ", ";
    }

    // int* numberCounter = new int[highestNumber];
    std::vector<int> numberCounter(highestNumber, 0);
    std::cout << "\nFrequency:\n";

    /*
    for (int i = 1; i <= highestNumber; i  ) {
        int temp2 = randomNumbers[i];
        numberCounter[temp2 - 1] = numberCounter[temp2 - 1]   1;
    }
    */

    // count frequencies no need to do intermediate counts of offsets in you 
    for (const int value : randomNumbers)
    {
        numberCounter[value]  ;
    }

    /*
    for (int i = 1; i <= highestNumber; i  ) {
        int frequency = 0;
        frequency = numberCounter[i] / outputLength;
        cout << numberCounter[i] << " occurs " << frequency << " of the time\n";
    }
    */

    // when you're going to divide numbers to show fractions
    // you will need a floating point type. 
    // in this case we do use indices since we need values from one array
    // to look into values of another array.
    double size = static_cast<double>(outputLength);
    for (std::size_t n = 0; n < highestNumber;   n)
    {
        double frequency = static_cast<double>(numberCounter[n]) / size;
        std::cout << n << " has a chance of " << frequency << " of occuring\n";
    }
}
  • Related