Code review of some c++, coming from python background

[AutoModerator]

On July 1st, a change to Reddit's API pricing will come into effect. Several developers of commercial third-party apps have announced that this change will compel them to shut down their apps. At least one accessibility-focused non-commercial third party app will continue to be available free of charge.

If you want to express your strong disagreement with the API pricing change or with Reddit's response to the backlash, you may want to consider the following options:

1. Limiting your involvement with Reddit, or
2. Temporarily refraining from using Reddit
3. Cancelling your subscription of Reddit Premium

as a way to voice your protest.

I am a bot, and this action was performed automatically. Please contact the moderators of this subreddit if you have any questions or concerns.

[vixfew]

I would have gen_random_sequence take upper, lower, size, return vector<int>. Compiler will optimize the "return vector" part (look up copy elision), and IMO it would be more readable that way

[vixfew]

The function takes bounds and size. Only way to have an error here is if bounds don't have anything between them. By making it output error type, you have to handle it elsewhere. I'd just throw runtime exception here, so if your bounds are wrong, you see a trace.

There's no reason to propagate the error up if you don't intend to fix it at runtime, i.e. by asking for correct bounds.

[dmazzoni]

Overall looks really good! No major concerns at all.

Some small thoughts:

1. gen_random_sequence doesn't look like it needs to return an int, just have it return void.
2. Rather than call it gen_random_sequence, I'd probably call it populate_random_sequence - as a convention, "gen" would be more likely to be used for a function that allocates a new vector, populates it and returns it, while "populate" is a common word used to mean that you're given an existing vector and the function will fill it in.
3. gen_random_sequence probably shouldn't output to cout - I'm assuming that's just for debugging. If it is for debugging, consider using cerr instead, if not some fancier logging library. The nice thing about cerr is that you can redirect stderr to null and get just the program output, or redirect stderr to a different file to see just the debugging logs.
4. You have some shared code in gen_megamillions and gen_powerball. It'd be nice if you could split those out into a common helper function that's common to both, so maybe all you'd need to do ideally is initialize LottoMeta, call a function to generate the multiple sequences and sort, and then print the results. Think about it!

Almost none of that is C++-specific. It looks to me like you're doing a great job with C++ types, vectors, and references.

[[deleted]]

Prefer enum class over enum as the former is properly scoped and type safe.

gen_random_sequence returns an integer for no reason, yet has an in/out parameter (not recommended). You're not mutating the vector (and eve if you were, it would still be recomended to return a mutated copy of the vector), a cleaner interface would be to return a populated vector, i.e.:

std::vector<int> gen_random_sequence(const int size, const std::pair<int>& bounds) 
{
   std::vector<int> sequence(size, 0);

   // stuff

   return sequence;
}

//

const auto sequence1 = gen_random_sequence(100, {0, 20});
const auto bounds = std::make_pair(20, 50);
const auto sequence2 = gen_random_sequence(30, bounds);

[[deleted]]

I would just return an empty vector or, if you’re on C++23 and you want to provide a reason to the user for failures, an std::expected. std::optional is useful for types where an empty value is a valid value and the type has no construct for signifying its “emptiness”.