2

My code generates a random number between 1 and 6 for 1000 times and I need to count how many times each number appeared in percentage. My problem is that it is, sometimes, generating negative percentages and it is surpassing 100%. I also must use a switch.

#include <stdio.h>
#include <time.h>
#include <stdlib.h>

int main(void) {
    int i, n, max = 6;
    int num1, num2, num3, num4, num5, num6;
    float total1, total2, total3, total4, total5, total6;

    srand(time(NULL));
    for (i = 0; i < 1000; i++) {
        n = rand() % max + 1; 
        switch (n) {
          case 1:
            num1 += 1;
          case 2:
            num2 += 1;
          case 3:
            num3 += 1;
          case 4:
            num4 += 1;
          case 5:
            num5 += 1;
          case 6:
            num6 += 1;
        }
    }
    total1 = num1 / 100;
    printf("Times 1 appeared: %f\n", total1);
    total2 = num2 / 100;
    printf("Times 2 appeared: %f\n", total2);
    total3 = num3 / 100;
    printf("Times 3 appeared: %f\n", total3);
    total4 = num4 / 100;
    printf("Times 4 appeared: %f\n", total4);
    total5 = num5 / 100;
    printf("Times 5 appeared: %f\n", total5);
    total6 = num6 / 100;
    printf("Times 6 appeared: %f\n", total6);
}
chqrlie
  • 131,814
  • 10
  • 121
  • 189
Nicholas
  • 21
  • 1
  • 2
    The `switch` code is missing `break;` statements. So when say `1` is rolled, *all* the `case` statements will execute, and all the counters will increment. Note too, that `num1/100` is doing *integer division* so there is no point assigning the result to a `float`. You need `num1/100.0` (or even `num1/10.0`) – Weather Vane Aug 15 '21 at 17:24
  • 1
    Why not use an array to keep track of the frequency of each number instead? – alex01011 Aug 15 '21 at 17:31
  • ... to obtain the percentage (which is `num * 100 / 1000.0`) – Weather Vane Aug 15 '21 at 17:31
  • @SteveSummit Or the instructor is clever by introducing one concept at a time. If the next topic of the class is to reduce repetetive code, that's a good lesson, and the switch statement is an obvious candidate for that. – Roland Illig Aug 15 '21 at 19:15

2 Answers2

2

There are several problems:

  • Several values were not initialized to 0. This needs to happen explicitly.
  • The switch cases had fallthrough. Landing on, e.g., case 1 meant that the execution continued through the other cases below it.
  • The totals were computed using integer division instead of floating point division, leading to a lack of precision.
  • The decimal point for the totals was in the wrong place.

The improved code:

#include <stdio.h>
#include <time.h>
#include <stdlib.h>

int main(void) {
    int i, n, max = 6;
    int num1 = 0, num2 = 0, num3 = 0, num4 = 0, num5 = 0, num6 = 0;    
    float total1 = 0.0F, total2 = 0.0F, total3 = 0.0F, total4 = 0.0F, total5 = 0.0F, total6 = 0.0F;

    srand(time(NULL));
    for (i = 0; i < 1000; i++) {
        n = rand() % max + 1; 
        switch (n)
        {
        case 1:
            num1 += 1;
            break;
        case 2:
            num2 += 1;
            break;
        case 3:
            num3 += 1;
            break;
        case 4:
            num4 += 1;
            break;
        case 5:
            num5 += 1;
            break;
        case 6:
            num6 += 1;
            break;
        }
    }
    total1 = num1/10.0F;
    printf("Times 1 appeared: %f\n", total1);
    total2 = num2/10.0F;
    printf("Times 2 appeared: %f\n", total2);
    total3 = num3/10.0F;
    printf("Times 3 appeared: %f\n", total3);
    total4 = num4/10.0F;
    printf("Times 4 appeared: %f\n", total4);
    total5 = num5/10.0F;
    printf("Times 5 appeared: %f\n", total5);
    total6 = num6/10.0F;
    printf("Times 6 appeared: %f\n", total6);
}

The code could be much more compact while achieving the same results. In C17:

#include <stdio.h>
#include <stdlib.h>
#include <time.h>

#define NUMBER_OF_BINS 6

int main(void)
{
    unsigned int number_of_iterations = 1000U;
    unsigned counters[NUMBER_OF_BINS] = {0U};

    srand(time(NULL));
    for (unsigned i = 0; i < number_of_iterations; ++i)
        ++counters[rand() % NUMBER_OF_BINS];

    for (size_t idx = 0; idx < NUMBER_OF_BINS; ++idx)
        printf("Times %zu appeared: %f\n", idx + 1, counters[idx] * 100.0F / number_of_iterations);
}
Yun
  • 3,056
  • 6
  • 9
  • 28
  • Thank you a lot! It worked now :) . I thought of using an array too, but the exercise needed a switch... – Nicholas Aug 15 '21 at 18:17
  • 1
    `%lu` might not be appropriate for `idx + 1` that has type `size_t`. The correct conversion specifier is `%zu`, but some C libraries are still not C99 compliant, so you should just use `int` for the index values and `%d`. – chqrlie Aug 15 '21 at 19:00
  • Good catch! Thank you. I've updated the answer. – Yun Aug 15 '21 at 19:13
  • The random numbers are biased. https://stackoverflow.com/questions/10984974 – Roland Illig Aug 15 '21 at 19:18
  • 1
    @RolandIllig True. I consider it outside the scope of this question/answer. – Yun Aug 15 '21 at 19:32
1

A better approach would be to use an array of ints to store the frequency of each number, if you could avoid the use of switch of course ,since you are dealing with a small range , 1 - 6, it can save you a lot of time. Should look like this,

#include <stdio.h>
#include <stdlib.h>
#include <time.h>

#define MAX 6

int main(void)
{
    int i, n;
    int num[MAX] = {0}; /* initialize to 0 */

    srand(time(NULL));

    for (i = 0; i < 1000; i++)
    {
        n = rand() % MAX + 1;

        num[n - 1]++; /* since arrays start from 0 , decrement by one to avoid overflow */
    }

    for (int i = 0; i < ΜΑΧ; i++)
    {
        printf("number %d, appeared :  %.1f%%\n", i + 1, ((float)num[i] * 100.0f / 1000.0f));
    }

    return 0;
}
alex01011
  • 1,670
  • 2
  • 5
  • 17