Assignment 1 #19
Assignment 1 #19
Conversation
…U-CSCD371-2025-Fall into assignment-1-colab
…U-CSCD371-2025-Fall into assignment-1-colab
…estion generation
There was a problem hiding this comment.
Pull Request Overview
This PR adds AI-generated trivia questions to the Princess Bride trivia game using OpenAI's API. After completing the preset questions, the game continues with dynamically generated questions until the user types 'exit'.
- Integrated OpenAI API to generate additional Princess Bride trivia questions
- Added continuous gameplay loop with exit condition
- Fixed integer division bug in percentage calculation
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Program.cs | Added OpenAI integration, continuous gameplay loop, and fixed percentage calculation |
| PrincessBrideTrivia.csproj | Added OpenAI package dependency |
| ProgramTests.cs | Added basic tests for AI question generation functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| { | ||
| var apiKey = Environment.GetEnvironmentVariable("OPENAI_API_KEY"); | ||
| Question q = await TriviaGenerator.GeneratePrincessBrideQuestionAsync(apiKey); | ||
| Assert.HasCount(4, q.Answers); |
There was a problem hiding this comment.
Assert.HasCount is not a valid MSTest assertion method. Use Assert.AreEqual(4, q.Answers.Length) instead.
| Assert.HasCount(4, q.Answers); | |
| Assert.AreEqual(4, q.Answers.Length); |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
This may have been the standard previously, but with the MSTests that were provided which are more current than the old standards, this is no longer the case, and .HasCount is actually correct.
| } | ||
|
|
||
| public static bool AskQuestion(Question question) | ||
| public static (bool, bool) AskQuestion(Question question) |
There was a problem hiding this comment.
Could replace the tuple with [flag] enum for better code clarity.
EX:
[Flags] public enum SorryImBadAtNames: byte { None = 0, AnswerdCorrectly = 1, QuitProgram = 2, };
Then you can use the objectEnum.HasFlag(FLAG); to figure out the response. More info at link:
https://learn.microsoft.com/en-us/dotnet/api/system.enum.hasflag?view=net-9.0
There was a problem hiding this comment.
Implementing enum would be a lot messier and a less concise. However, dealing with both the correct answers and quitting the program in one function is a good suggestion and will be implemented.
There was a problem hiding this comment.
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 ✔
Very impressive in my opinion! The AI feature is super cool and you guys definitely have some know how! great work
| string answer2 = lines[lineIndex + 2]; | ||
| string answer3 = lines[lineIndex + 3]; | ||
| Question question = new() { Text = lines[lineIndex], Answers = new string[3], CorrectAnswerIndex = lines[lineIndex + 4] }; | ||
| question.Answers[0] = lines[lineIndex + 1]; |
There was a problem hiding this comment.
I like how in this section you condensed the code and made it more streamlined. However, I personally prefer the readability of the previous version. Since I don't see this contradicting any guidelines this is purely a stylistic opinion.
There was a problem hiding this comment.
I'd personally chose to stick with the streamlined code because it’s cleaner and easier to maintain. It does the same thing as before but with less repetition, which reduces the chance of errors if future maintenance was to become a concern.
| if (string.IsNullOrWhiteSpace(apiKey)) | ||
| throw new ArgumentException("API key is required.", nameof(apiKey)); | ||
|
|
||
| if (choices is < 3 or > 6) |
There was a problem hiding this comment.
Very much grasping for straws here cause your code is so spot on! perhaps you should avoid making the bounds magic numbers and instead having the answer bounds be variables or consts. This would add versatility if the program ever was to be updated with more options
There was a problem hiding this comment.
Thanks for the feedback! In this case, I chose to leave the bounds as-is since the program is intentionally simple and unlikely to change. Introducing constants or variables here wouldn’t add much value for this particular use case, but I appreciate the suggestion.
There was a problem hiding this comment.
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 ✔
Cool feature update!
…U-CSCD371-2025-Fall into assignment-1-colab
…e Assert.HasCount(...) instead of Assert.AreEqual(..., collection.Length).
…n the same method in order to have more readable code.
… of the program. I changed it to the very beginning so as to be more user friendly. I also fixed the grades to display properly- whether you quit early, or if you finish everything correctly.
SummarySummary
CoveragePrincessBrideTrivia - 22.1%
|
I worked with @JosephPotapenko on this assignment.
For extra credit, we implemented an AI trivia question generator that uses the OpenAI API to generate more questions once the preset ones run out. In order to use this feature, the
OPENAI_API_KEYenvironmental variable must be set to a valid API key.