I'm a beginner in C++ and I'm working on a personal project to improve my coding skills. I've created a simple dice function for a monopoly-themed video game, using a Linear Congruential Generator I found online to create pseudo-random numbers, along with some conditional statements. I also learned how to utilize tuples in C++ to return both the calculated value and the dice outcome. I'd appreciate any feedback on my code and suggestions on how to improve it! Here's the code:
```cpp
#include
#include
#include
std::tuple LCGDice(double m, double a, double c, double seed) {
double calc {std::fmod((a * seed + c), m)}; // Calculation of LCG value
double mDivision = m / 6.0; // Divide "m" by 6
if (calc >= 0 && calc <= mDivision) {
std::cout <= mDivision && calc <= mDivision * 2.0) {
std::cout <= mDivision * 2.0 && calc <= mDivision * 3.0) {
std::cout <= mDivision * 3.0 && calc <= mDivision * 4.0) {
std::cout <= mDivision * 4.0 && calc <= mDivision * 5.0) {
std::cout << "DICE VALUE: 5n";
return std::make_tuple(calc, 5.0);
} else {
std::cout << "DICE VALUE: 6n";
return std::make_tuple(calc, 6.0);
}
}
int main() {
std::cout << "LGF DICE FUNCTION" << std::endl;
double m{std::pow(2.0, 32.0)};
double a{1664525};
double c{1013904223};
double seed{1};
double calculation{1};
double dice{};
for (double i{seed + 1}; i <= 10.0; ++i) {
std::tie(calculation, dice) = LCGDice(m, a, c, calculation);
}
return 0;
}
```
What do you think?
3 Answers
Great job for a beginner! You've tackled pseudo-random generation and the use of tuples, which are important programming concepts. Your implementation generally looks solid, especially the way you return both the seed and dice value. A couple of improvements to consider: 1) Instead of using doubles for dice values, consider using integers since a dice value is always a whole number. 2) Think about exploring the `` library once you're comfortable. 3) You might have some overlapping conditions in your if statements. For instance, both the first and second conditions can trigger if `calc` is exactly equal to `mDivision`. Also, there's no need to use doubles in your for loop in the main function.
One suggestion I have is about naming conventions. Instead of using acronyms like 'LCG' in your code, it’s helpful to spell them out somewhere. This prevents future confusion, especially if other developers (or even yourself later on) forget what it stands for. Also, consider renaming the parameters 'm', 'a', and 'c' to something more descriptive so their purpose is clear. Furthermore, it might be useful to separate the linear congruential generator logic from the dice rolling logic; this would make your code cleaner and easier to debug.
Good point! I'll rename the parameters for clarity and think about splitting the LCG from the dice logic.
To add to the previous feedback, a good practice is to avoid using `else` after a return statement. If you refactor your if conditions without using else, you'll simplify the code and reduce the risk of bugs. Simplifying your logic makes your intentions clearer and your code easier to read. Keep coding and experimenting with these improvements, it'll help you grow as a programmer!
Thanks for your feedback! I'll definitely work on using integers and check the conditions as you suggested!