My implementation of the MathGame project.#723
Conversation
There was a problem hiding this comment.
@BudgetKratos Thanks for submitting! There's a bug blocking in the history, which blocks the approval, could you please check?
When the player chooses not to play again, Operate() calls Menu() before reaching ScoreCard.Add(...). Consequently, the completed result is not added to the score history until the nested menu eventually returns. Repeated navigation also creates an unnecessary chain of recursive method calls.
Instead of calling Menu() from inside Operate(), let Operate() finish naturally:
if (!retry)
{
ScoreCard.Add($"{CurrentDifficulty} {actualOperation} {score}");
DisplayMessage("Press any key to go back to the previous menu.", displayColor);
Console.ReadKey();
return;
}
Upon fixing, just commit your changes, no need for a new PR 😁
|
Good catch, thank you ^_^ I've also slightly updated the code so that every retry counts as a separate attempt (as I had originally intended). |
|
@BudgetKratos Project approved! 😄✅ And well done for completing the challenges! I recommend you always do that, as it pushes you and makes you learn faster. Your project is clean, tidy, concise and to the point. And you're using only one method for all operations, great work! 🥋And congratulations! You got your green belt!! 🟩🟩 Feedback💖You showed good use of inheritance, separation of concerns and nice helper methods. 👏👏 🔍️1. Avoid storing scores as strings. This is probably my biggest suggestion. Currently: Later you split it again. Instead, create a small model. class Score Then This removes parsing entirely and is much more object-oriented. 🔍️ Avoid comparing enum names as strings Instead of compare the enum directly. or even better, use a switch expression. This is safer, faster and much more idiomatic C#. 🔍️ Reduce repeated switch logic. The operation selection contains several repeated patterns. For example A dictionary or small configuration object could map operation colour and title to reduce maintenance if you ever add more games. 🔍Random can be non-nullable: Since it's initialized immediately the nullable annotation isn't necessary. 🔍CheckAnswer can be simplified. A switch expression makes it shorter, cleaner and easier to extend. 🗺️ I noticed that the only course you've completed so far is Introduction to Object-Oriented Programming. I'd highly recommend taking our LINQ Fundamentals course next. LINQ is one of the most important tools in modern C# and becoming comfortable with it will make your code much cleaner and more expressive. ☕ If you like our roadmap, please consider buying us a coffee. We appreciate your help 🙂 Overall great work! ✋🏻 Looking forward to seeing your next projects! |
I took the advice below a bit too literally, this was my third attempt at the Math Game.
I'm sure if I were left to my own devices, I would still be obsessing over the tiny details, trying to make everything perfect.
It's a habit I should kick or, at the very least, channel into something else!