HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Virtual Lotto Task

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
lottovirtualtask

Problem

I had the task to write a Lotto simulator. My program works as follows:

  • To start, the user can type in 6 numbers.



  • Then the winning numbers are randomly chosen.



  • The program then shows the winning numbers followed by the numbers the user chose at the beginning.



  • Finally, the user has the option if he wants to play again, where they can use their old numbers or new ones.



If the user types in more than one number at a time, the program should close.
If the user types in a number that isn't between 1 and 49, the user should have the option to try again.

```
#include // for cin and cout streams
#include // for the rand and srand functions
#include
#include
#include
#include
#include
#include

using namespace std;

vector numbers;

bool askYN()
{
string YN;
cin >> YN;
return YN == "y";
}

void askSameNumber() {
cout getRandoms() {
vector res;
srand(time(0));
for (int i = 0; i in_avail() != 0) {
string n;
stream >> n;
if (is_number(n)) {
numbers.push_back(std::stoi(n));
}
else {
std::cout 48) {
throw myex;
}
}
return;
}

void body() {
try {
if (numbers.size() != 0){
askSameNumber();
}
if (numbers.size() != 6){
askNumbers();
}
vector randoms;
randoms = getRandoms();
cout << "winning numbers: ";
for (int i = 0; i < randoms.size(); i++) {
cout << randoms[i] << " ";
}
cout << "\n";
for (int i = 0; i < randoms.size(); i++) {
if (std::find(randoms.begin(), randoms.end(), numbers[i]) != randoms.end()) {
cout << "your number " << numbers[i] << " win\n";
}
}
cout << "do you want to play again? [y/n]: ";
if (askYN()) {
body();
}

}
catch (exception e) {
cout << "try again: \n";
body();
}
}

int main()
{
b

Solution

First, consider using standard algorithms to avoid writing error-prone, boilerplate code. Second, in my opinion, some of your functions do too much (remember, "one function, one responsibility"). For example, you have a function that reads input and modifies the contents of a container. Also, I feel like the hiding the functionality of the main function behind body only hurts readability.

My improved version below, and then some comments.

#include 
#include 
#include 
#include 
#include 
#include 

std::vector draw_numbers() 
{
    srand(std::time(0));

    std::vector randoms;
    std::generate_n(std::back_inserter(randoms), 6, []() { return rand() % 48 + 1; });
    std::sort(randoms.begin(), randoms.end());

    return randoms;
}

bool is_number(const std::string& s)
{
    return !s.empty() && find_if(s.begin(),
        s.end(), [](char c) { return !isdigit(c); }) == s.end();
}

bool is_number_in_range(const std::string& s)
{
    return is_number(s) && std::stoi(s) >= 1 && std::stoi(s)  ask_numbers()
{
    std::vector v(1, ".");

    while (!std::all_of(v.begin(), v.end(), is_number_in_range))
    {
        v.clear();
        std::cout (std::cin), 6, std::back_inserter(v));
    }

    std::vector nums;
    std::transform(v.begin(), v.end(), std::back_inserter(nums), [](const std::string& c) { return std::stoi(c); });

    return nums;
}

int main()
{
    char ans_again = 'n';
    std::vector numbers = ask_numbers();

    do
    {
        std::vector randoms = draw_numbers();

        std::cout (std::cout, " "));
        std::cout  hits;
        std::set_intersection(numbers.begin(), numbers.end(), randoms.begin(), randoms.end(), std::back_inserter(hits));

        for (int h : hits)
        {
            std::cout > ans_again;

        if (ans_again == 'y')
        {
            std::cout > ans_same;

            if (ans_same == 'n')
                ask_numbers();
        }

    } while (ans_again == 'y');
}


-
You might also prefer random number generation from C++ instead of the C rand.

-
The code quite heavily uses standard algorithms. In addition, it's trying to avoid "handwritten" ifs and loops (after all, such places often contain those nasty bugs).

-
Avoid global variables, it will become a nightmare to maintain code with them. Instead, make all variables as local as possible. It also helps readability.

-
The code above is using some "magic constants" like 1, 48, and 6. Perhaps it's a nice exercise to make then constant values so that you can experiment with different kinds of lotto machines easily.

Code Snippets

#include <iostream>
#include <cstdlib>
#include <vector>
#include <ctime>
#include <string>
#include <algorithm>

std::vector<int> draw_numbers() 
{
    srand(std::time(0));

    std::vector<int> randoms;
    std::generate_n(std::back_inserter(randoms), 6, []() { return rand() % 48 + 1; });
    std::sort(randoms.begin(), randoms.end());

    return randoms;
}

bool is_number(const std::string& s)
{
    return !s.empty() && find_if(s.begin(),
        s.end(), [](char c) { return !isdigit(c); }) == s.end();
}

bool is_number_in_range(const std::string& s)
{
    return is_number(s) && std::stoi(s) >= 1 && std::stoi(s) <= 48;
}

std::vector<int> ask_numbers()
{
    std::vector<std::string> v(1, ".");

    while (!std::all_of(v.begin(), v.end(), is_number_in_range))
    {
        v.clear();
        std::cout << "Input 6 numbers between 1 and 48 inclusive: ";
        std::copy_n(std::istream_iterator<std::string>(std::cin), 6, std::back_inserter(v));
    }

    std::vector<int> nums;
    std::transform(v.begin(), v.end(), std::back_inserter(nums), [](const std::string& c) { return std::stoi(c); });

    return nums;
}

int main()
{
    char ans_again = 'n';
    std::vector<int> numbers = ask_numbers();

    do
    {
        std::vector<int> randoms = draw_numbers();

        std::cout << "Winning numbers: ";
        std::copy(randoms.begin(), randoms.end(), std::ostream_iterator<int>(std::cout, " "));
        std::cout << "\n";

        std::sort(numbers.begin(), numbers.end());
        std::vector<int> hits;
        std::set_intersection(numbers.begin(), numbers.end(), randoms.begin(), randoms.end(), std::back_inserter(hits));

        for (int h : hits)
        {
            std::cout << "Your number " << h << " wins\n";
        }

        std::cout << "Do you want to play again? [y/n]: ";
        std::cin >> ans_again;

        if (ans_again == 'y')
        {
            std::cout << "Do you want to use the same numbers? [y/n]: ";
            char ans_same = 'n';
            std::cin >> ans_same;

            if (ans_same == 'n')
                ask_numbers();
        }

    } while (ans_again == 'y');
}

Context

StackExchange Code Review Q#131162, answer score: 2

Revisions (0)

No revisions yet.