4
\$\begingroup\$

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 <iostream> // for cin and cout streams
#include <cstdlib>  // for the rand and srand functions
#include <vector>
#include <sstream>
#include <exception>
#include<ctime>
#include<string>
#include<algorithm>


using namespace std;

vector<int> numbers;

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

void askSameNumber() {
    cout << "do you want to use the same number? [y/n]\n";
    if (!askYN()) {
        numbers.clear();
    }
};

class myexception : public exception
{ } myex;


vector<int> getRandoms() {
    vector<int> res;
    srand(time(0));
    for (int i = 0; i < 6; i++){
        int b = rand() % 48 + 1;
        res.push_back(b);
    }
    return res;
}
bool is_number(const string& s)
{
    return !s.empty() && find_if(s.begin(),
        s.end(), [](char c) { return !isdigit(c); }) == s.end();
}

void askNumbers() throw(myexception){
    cout << "input 6 numbers between 1 and 48 inclusive\n";
    char input[100];
    cin.getline(input, sizeof(input));
    stringstream stream(input);
    while (stream.rdbuf()->in_avail() != 0) {
        string n;
        stream >> n;
        if (is_number(n)) {
            numbers.push_back(std::stoi(n));
        }
        else {
            std::cout << "Not valid input. Provide number" << std::endl;
        }
    }
    cout << "size " << numbers.size() << "\n";
    if (numbers.size() != 6) {
        throw myex;
    }
    for (int i = 0; i < 6; i++) {
        if (numbers[i] < 0 || numbers[i] > 48) {
            throw myex;
        }
    }
    return;
}


void body() {
    try {
        if (numbers.size() != 0){
            askSameNumber();
        }
        if (numbers.size() != 6){
            askNumbers();
        }
        vector<int> 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()
{
    body();
    return 0;
}
\$\endgroup\$
2
\$\begingroup\$

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 <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');
} 
  1. You might also prefer random number generation from C++ instead of the C rand.

  2. 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).

  3. 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.

  4. 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.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.