-
Notifications
You must be signed in to change notification settings - Fork 517
Finished Math Game #636
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Finished Math Game #636
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rumanstheddy Thanks for submitting! The project is looking good but there a couple of things I need you to address before I can approve. These aren't requirements but they'll be a good learning opportunity for you:
1️⃣The game is very limited if you're hard coding the questions. You can easily generate Random questions in C#.
2️⃣At the moment there's no input validation, so if I input something wrong the app crashes. Don't use recursion, use a loop for the validation.
3️⃣Your ShowMenu() calls StartGame(). StartGame() calls ShowMenu(). That's an endless recursion that means that each time a method is called it stays in the stack, occupying a little extra memory. While this is not a big issue in a small app, a few more methods and collections would quickly add up to a stack overflow. For that reason recursion isn't good practice in most cases.
Could you please fix these before I can approve? 😀Let me know if you have questions.
|
Thank you so much for taking the time to review these changes! |
…and remove static question dictionary
… and flow control
|
@TheCSharpAcademy, I have implemented the fixes you suggested. |
TheCSharpAcademy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rumanstheddy Thanks for making the changes! There's still one problem. The divisions are crashing sometimes. Try playing the division game a few times and see if your app stops like this:

I suspect there's something wrong with the ranges of your division when a = 1 or a = 2.

|
@TheCSharpAcademy Thank you so much for reviewing the code again! |
No description provided.