(Dog) Agility - Penalties Calculator

Question

I have written the following code. I hope to improve how it completes the given task, since I think that my code - even if "properly working" - is quite "brute" and it could greatly use some improvement.

The exercise is about Agility (dogs) and it wants me to calculate, for a total of 9 pairs (dog+human), the total penalties (extracted from the probability that they will either refuse to clear an obstacle and/or make a mistake while clearing an obstacle) for each dog, for a total of 20 obstacles. Specifically, a dog cannot refuse to do an obstacle if he has already tried it and made a mistake, nor can he be penalized twice for the same refusal/mistake.

Supposedly there's a special obstacle - the slalom - that circumvents the rules: if a dog makes a mistake while performing the slalom, it can then also refuse to go through it (and this would result in 2 points of penalty). My code does not consider this special obstacle.

(The probability of the dogs refusing/making a mistake is supposed to be 5%)

//Agility

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

#define PAIRS 9
#define OBSTACLES 20

int main(void)
{
    int refusal[PAIRS] = {0}, error[PAIRS] = {0};
    srand(time(NULL));

    //Print header
    printf("| %4s | %8s | %6s | %9s |\n", "Pair", "Refusals", "Errors", "Penalties");

    for (int i = 0; i < PAIRS; ++i) {
        //Calculate refusals
        for (int o = 0; o < OBSTACLES; ++o) {
            int temp_refusal_2[OBSTACLES];
            int temp_refusal = 1 + rand() % 100;

            if (temp_refusal > 0 && temp_refusal < 6) {
                temp_refusal_2[o] += 1;

                if (temp_refusal_2[o] == 1) {
                        refusal[i] += 1;
                }
            }else { //Calculate errors
                int temp_error_2[OBSTACLES];
                int temp_error = 1 + rand() % 100;

                if (temp_error > 0 && temp_error < 6) {
                    temp_error_2[o] += 1;

                    if (temp_error_2[o] == 1) {
                        error[i] += 1;
                    }
                }
            }
        }   
        //Print summary
        printf("| %4d | %8d | %6d | %9d |\n", (i + 1), refusal[i], error[i], (refusal[i] + error[i]));
    }
    puts("");

    return 0;
}

Answer 1

alternative ways of completing the given task,

Post error and warning free code.

---

Enable all compiler warnings

I do not believe OP has posted working code.

// warning: format '%s' expects argument of type 'char *', but argument 2 has type 'int' [-Wformat=]
printf("| %4s | %8s | %6s | %9s |\n", (i + 1), refusal[i], error[i],
    (refusal[i] + error[i]));

// warning: conversion from 'time_t' {aka 'long int'} to 'unsigned int' may change value [-Wconversion]
srand(time(NULL));

---

Uninitialized values

temp_error_2[o] += 1 attempts to increment an int that have never been initialized.

            int temp_error_2[OBSTACLES];
            int temp_error = 1 + rand() % 100;

            if (temp_error > 0 && temp_error < 6) {
                temp_error_2[o] += 1;  // Bad!!

Unneeded array

Array int temp_error_2[OBSTACLES]; not needed.

            // Weak
            int temp_error_2[OBSTACLES];
            int temp_error = 1 + rand() % 100;

            if (temp_error > 0 && temp_error < 6) {
                temp_error_2[o] += 1;

                if (temp_error_2[o] == 1) {
                    error[i] += 1;
                }
            }

Replaceable with

            int temp_error = 1 + rand() % 100;

            if (temp_error > 0 && temp_error < 6) {
              error[i]++;
            }

---

Too many language and logic errors to continue review.

Answer 2

This code is at the limit of something that can be contained in main(). If it were any larger, I would insist that it should be divided into functions.

Build it with a good set of warnings enabled. I compiled with gcc -Wall -Wextra -Wwrite-strings -Wno-parentheses -Wpedantic -Warray-bounds -Wmissing-braces -Wconversion -Wstrict-prototypes -fanalyzer and that reveals the need for a cast of the argument to srand() and a failure to initialise the array variables temp_refusal_2 and temp_error_2.

We have two very similar code blocks where we get a random value that's 5% likely to be true - that's a candidate for a separate function, perhaps like this:

static int rand_five_percent(void)
{
    return rand() % 20 == 0;
}

It's not clear that we have correctly implemented this requirement:

(The probability of the dogs refusing/making a mistake is supposed to be 5%)

We correctly implement 5% of refusals (modulo a slight bias when RAND_MAX is not an exact multiple of 100), but then have 5% of those that do not refuse make an error. That might be what's required, or the requirement could be that 5% of the teams refuse and 5% of teams error. If we cannot confirm which is intended, we should at least document which interpretation we have chosen.

The array variables don't need to be arrays - we only ever use one index at a time, and never revisit previous teams or obstacles. So we can use simple integer variables for the counts.

Minor things:

• return 0; is optional for main() (but for no other functions). If we reach the end of the main function, it automatically returns 0 to indicate program success.
• It's arguably better to use unsigned types to model values that are necessarily non-negative. There are people who disagree with this, though.
• Prefer strongly-typed constant values, rather than preprocessor macro substitution, when possible.

---

Improved code

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

static const unsigned int Pairs = 9;
static const unsigned int Obstacles = 20;

static int rand_five_percent(void)
{
    return rand() % 20 == 0;
}

int main(void)
{
    srand((unsigned)time(NULL));
    printf("| %4s | %8s | %6s | %9s |\n",
           "Pair", "Refusals", "Errors", "Penalties");

    for (unsigned int i = 1;  i <= Pairs;  ++i) {
        unsigned int refusals = 0;
        unsigned int errors = 0;

        for (unsigned int o = 0;  o < Obstacles;  ++o) {
            if (rand_five_percent()) {
                /* 5% of teams refuse the obstacle */
                ++refusals;
            } else if (rand_five_percent()) {
                /* 5% of those that attempt it, fail */
                ++errors;
            }
        }

        printf("| %4u | %8u | %6u | %9u |\n",
               i, refusals, errors, (refusals + errors));
    }
}