-
Notifications
You must be signed in to change notification settings - Fork 22
Assignment 1 #16
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: Assignment1
Are you sure you want to change the base?
Assignment 1 #16
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.
Assignment Details
- Pull Request title includes "Assignment 1" in the title ✔
- Partner has also made a commit ✔
- Pull Request targets Assignment 1 branch ✔
- Issue 1: Application no longer crashes ✔
- Issue 2: Unit test properly passes ✔
Extra Credit
-
Come up with a new feature for the application
- Issue 3: Add a new feature request implemented ✔
- Issue 3: Feature request unit tested ✔
This hard mode is absolutely diabolical! Great work!
Assignment Details |
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.
Pull Request Overview
This PR adds a hard mode feature to the Princess Bride trivia game where users answer questions by typing the actual answer text instead of selecting multiple choice options. The implementation includes input validation, punctuation stripping, and case-insensitive comparison.
- Added hard mode selection prompt with input validation
- Modified question display to hide multiple choice options in hard mode
- Implemented text-based answer comparison with punctuation removal and case-insensitive matching
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
File | Description |
---|---|
global.json | Updated .NET SDK version from 9.0.100 to 9.0.305 |
Program.cs | Added hard mode functionality with user input validation, text-based answer comparison, and modified question display logic |
ProgramTests.cs | Added unit tests for the new DisplayHardResult method and removed extra whitespace |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
@BenjaminMichaelis I messed up my branch and working directory at the start and I didn't catch it before my collaborator joined up, they branched off of my working branch so I didn't want to squash the commits and risk losing those. I considered trying to cherry pick the commits to clean it all up but I'm just not familiar enough. |
…r must type out the answers instead of selecting them from a multiple choice sheet
decimal integer division
…er method to strip puncuation from user input
…ompt user if input is incorrect
corrected spelling of "arbitrary" Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Moved question object instantiation for safety
Added suggested validation check. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
860551e
to
5ddb020
Compare
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.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
Great work!
Assignment Details
Pull Request title includes "Assignment 1" in the title ✔
Partner has also made a commit ✔
Pull Request targets Assignment 1 branch ✔
Issue 1: Application no longer crashes ✔
Issue 2: Unit test properly passes ✔
Extra Credit
Come up with a new feature for the application
Issue 3: Add a new feature request implemented ✔
Issue 3: Feature request unit tested ✔
string normalizedGuess = RemovePunctuation(userGuess).Trim(); | ||
if (!int.TryParse(question.CorrectAnswerIndex, out int index) || index < 1 || index > question.Answers.Length) | ||
{ | ||
Console.WriteLine("Incorrect"); |
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.
Small nitpick, but here you simply write "Incorrect" when the user enters something that CorrectAnswerIndex can't parse or is out of range. This reads like they just got the answer wrong, when it's actually a parsing issue. It might be clearer to print a more descriptive message, "Error - user answered out of range" and prompt for new input, or throwing an exception on it.
Commit from a small change that copilot suggested Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Another commit for a suggestion by copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
SummarySummary
CoveragePrincessBrideTrivia - 50.4%
|
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.
Assignment Details
- Pull Request title includes "Assignment 1" in the title ✔
- Partner has also made a commit ✔
- Pull Request targets Assignment 1 branch ✔
- Issue 1: Application no longer crashes ✔
- Issue 2: Unit test properly passes ✔
Extra Credit
- Come up with a new feature for the application
- Issue 3: Add a new feature request implemented ✔
- Issue 3: Feature request unit tested ✔
} | ||
} | ||
|
||
// Unit test for hardmode |
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.
Comments describing things that can be determined by looking at the code clutter the codebase. I'll reference the guidelines in a sec, but I'd lean towards writing self-documenting code. Under the books guidelines, it says DO NOT use comments unless they describe something that is not obvious to someone other than the developer who wrote the code.
There are a few other places in the PR that show similar comments - I'll briefly note one other.
|
||
|
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.
I don't think the extra whitespace adds anything here
} | ||
|
||
public static bool AskQuestion(Question question) | ||
//UPDATED THIS FUNCTION WITH PARAMETER isHard |
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.
This one isn't necessary either for example
@c-stanton @BillMillerCoding remember to address (reply to), possibly fix, and resolve all peer review comments in future assignments. Copilot's review as well. I see some changes according to reviews, but not all are responded to, possibly (if you agree with the reviewer) fixed, and resolved. |
Collaborated with @BillMillerCoding
Added extra credit hardmode trivia feature.