1

So, I am having a problem with my code. The program needs to choose randomized one from 4 printfs and print it in the terminal. I am new at this so sorry about that.

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

int main () {
    setlocale (LC_ALL, "Portuguese");
    int opcao;
    opcao = rand() % 3 + 1;

    if (opcao == 0) {
        printf ("\nA opção sorteada foi a de que o 1º classificado atual será o campeão (FC Porto)");
    }

    if (opcao == 1) {
        printf ("\nA opção sorteada foi a de que o 1º classificado na 1ª volta será o campeão (SL Benfica)");
    }

    if (opcao == 2) {
        printf ("\nA opção sorteada foi a de que Porto e Benfica farão um jogo em campo neutro para determinar o campeão!");
    }

    if (opcao == 4) {
        printf ("\nFoi sorteada a opção de que não haverá campeão esta época");
    } 


    return 0;
}

This is my code but is only choosing the same printf ever and ever.

Allen Haley
  • 639
  • 5
  • 21

4 Answers4

2

Use the <random> library instead of the obsolete and error-prone std::rand (using the modulo operator to obtain a random integer in a range is a common mistake). See Why is the new random library better than std::rand()? for more information.

#include <iostream>
#include <random>

int main()
{
    std::mt19937 engine{std::random_device{}()};
    std::uniform_int_distribution<int> dist{0, 3};

    switch (dist(eng)) {
    case 0:
        std::cout << "...\n";
        break;
    case 1:
        std::cout << "...\n";
        break;
    case 2:
        std::cout << "...\n";
        break;
    case 3:
        std::cout << "...\n";
        break;
    }
}

Here, we first create a std::mt19937 engine, which produces uniformly distributed integers in the half-open range [0, 232), and seed it using a std::random_device, which is supposed to generate a non-deterministic number (it may be implemented using the system time, for example). Then, we create a std::uniform_int_distribution to map the random numbers generated by the engine to integers in the inclusive interval [0, 3] by calling it with the engine as argument.


This can be generalized by taking a range of strings to print:

template <typename RanIt, typename F>
decltype(auto) select(RanIt begin, RanIt end, F&& f)
{
    if (begin == end) {
        throw std::invalid_argument{"..."};
    }

    thread_local std::mt19937 engine{std::random_device{}()};

    using index_t = long long; // for portability
    std::uniforn_int_distribution<index_t> dist{0, index_t{end - begin - 1}};

    return std::invoke(std::forward<F>(f), begin[dist(engine)]);
}

int main()
{
    const std::array<std::string, 4> messages {
        // ...
    };
    select(messages.begin(), messages.end(),
           [](const auto& string) {
               std::cout << string << '\n';
           });
}

Here, we take a pair of random access iterators and a Callable object to support selecting an element from an arbitrary random-accessible range and performing an arbitrary operation on it.

  • First, we check if the range is empty, in which case selection is not possible and an error is reported by throwing an exception.

  • Then, we create a std::mt19937 engine that is thread_local (that is, each thread has its own engine) to prevent data races. The state of the engine is maintained between calls, so we only seed it once for every thread.

  • After that, we create a std::uniform_int_distribution to generate a random index. Note that we used long long instead of typename std::iterator_traits<RanIt>::difference_type: std::uniform_int_distribution is only guaranteed to work with short, int, long, long long, unsigned short, unsigned int, unsigned long, and unsigned long long, so if difference_type is signed char or an extended signed integer type, it results in undefined behavior. long long is the largest supported signed integer type, and we use braced initialization to prevent narrowing conversions.

  • Finally, we std::forward the Callable object and std::invoke it with the selected element. The decltype(auto) specifier makes sure that the type and value category of the invocation is preserved.

We call the function with an std::array and a lambda expression that prints the selected element.

Since C++20, we can constrain the function template using concepts:

template <std::random_access_iterator RanIt,
          std::indirectly_­unary_­invocable<RanIt> F>
decltype(auto) select(RanIt begin, RanIt end, F&& f)
{
    // ...
}

Before C++20, we can also use SFINAE:

template <typename RanIt, typename F>
std::enable_if_t<
    std::is_base_of_v<
        std::random_access_iterator_tag,
        typename std::iterator_traits<RanIt>::iterator_category
    >,
    std::invoke_result_t<F, typename std::iterator_traits<RanIt>::value_type>
> select(RanIt begin, RanIt end, F&& f)
{
    // ...
}
L. F.
  • 19,445
  • 8
  • 48
  • 82
  • Nice use of modern C++ here. While it does not point out the direct bug in the original question, this is a really good example on how to use the standard libraries. – smithco Mar 18 '20 at 22:04
  • Indeed using the new Random library is better! I would add some more explanation for the second example, since it is not very simple to understand, especially for the user who asked the question. – Kerek Mar 20 '20 at 01:25
  • 1
    @Kerek I've added some explanation and a bunch of links. Better now? – L. F. Mar 20 '20 at 02:19
  • Perfect! :) Thanks for the clarifications! – Kerek Mar 20 '20 at 22:55
1

You did not provide a seed for the random number generator. From the man page,

If no seed value is provided, the functions are automatically seeded with a value of 1.

If you have the same seed on every run, you'll always get the same random sequence.

smithco
  • 679
  • 5
  • 17
1

A couple of problems with your program:

  • You don't have a seed, that's the reason why the numbers are repeated. Use
srand (time(NULL)); // #include <time.h>

before you use rand()

  • Your random numbers are not sequenced,you have 0-2 and then 4, when you get 3 there is no option available. If it's on purpose, ignore this remark.

  • With rand() % 3 + 1; your random numbers will range from 1 to 3, so opcao == 0 and opcao == 4 will never occur. For a 0-4 interval you will need something like:

 opcao = rand() % 5;
anastaciu
  • 23,467
  • 7
  • 28
  • 53
1

I wanted to add some things here, adding to the previous answers.

First and foremost, you are not writing the code for yourself. As a non-native English speaker, I understand why it may seem easier to write down a code in your native language, but don't do it!

Secondly, I have made changes to the code in order to make it easier and more readable:

#include <time.h>
#include <cstdint>
#include <iostream>
#include <string>

constexpr uint32_t NUM_OF_POSSIBLE_PROMPTS = 4;

int main () {
    srand(time(NULL)); // seed according to current time

    for (uint32_t i = 0; i < 10; ++i)
    {
        int option = rand() % (NUM_OF_POSSIBLE_PROMPTS);

        std::string prompt = "";

        switch (option)
        {
            case 0:
                prompt = "option 0";
                break;
            case 1:
                prompt = "option 1";
                break;
            case 2:
                prompt = "option 2";
                break;
            case 3:
                prompt = "option 3";
                break;
            default:
                // some error handling!
                break;
        }

        std::cout << prompt << std::endl;
    }

    return 0;
}
  1. I'm using switch-case instead of if-else-if-else which is much more readable and efficient!

  2. I'm using constexpr in order to store my hard-coded number - it is a bad habit having hardcoded numbers in the code (in a real program, I would make a constexpr value of 10 as well for the loop boundaries).

  3. In c++ (unlike in c), we use std::cout and its operator << in order to print, and not the printf function. It makes a unified behavior with other types of streams, such as stringstream (which is helpful when trying to construct strings in real time, however, it is somewhat heavy on resources!).

Since this code is more organized, it easier for you to understand where a possible error may occur, and also makes it less likely for that to happen in the first place.

For example, using gcc's -Wswitch-enum flag will make sure that if you use an enum, all values must be handled in switch-case sections (which makes your program less error prone, of course).

P.S, I have added the loop only to show you that this code is getting different results every round, and you can test this by running the code multiple times.

Kerek
  • 1,106
  • 1
  • 8
  • 18