Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
a59ac2f
casts numbers to float for correct percent calculation
chanderlud Sep 30, 2025
6405f02
Adding Questions into the array
JosephPotapenko Sep 30, 2025
0903549
Merge branch 'assignment-1-colab' of https://github.com/chanderlud/EW…
JosephPotapenko Sep 30, 2025
0024d32
WIP AI generated trivia questions
chanderlud Sep 30, 2025
8d49d27
Merge branch 'assignment-1-colab' of https://github.com/chanderlud/EW…
chanderlud Sep 30, 2025
3c4ff18
finished AI generated trivia questions & adds basic unit tests for qu…
chanderlud Sep 30, 2025
b728c81
Updated comments
chanderlud Sep 30, 2025
1ec8dca
disables question generation tests when an OpenAI API key is not avai…
chanderlud Sep 30, 2025
deb917e
improved error handling and flow
chanderlud Sep 30, 2025
ac8b0fe
refactors TriviaGenerator class to its own file
chanderlud Sep 30, 2025
a8d2e1b
simplifies LoadQuestions by removing temporary variables
chanderlud Sep 30, 2025
3c0106b
refactors to string interpolation, rounds the percent to two decimal …
chanderlud Sep 30, 2025
1a8065d
resolves several warnings
chanderlud Sep 30, 2025
6a134af
Fixing the .HasCount to .AreEqual in order to follow proper test syntax.
JosephPotapenko Oct 2, 2025
3148200
Merge branch 'assignment-1-colab' of https://github.com/chanderlud/EW…
JosephPotapenko Oct 2, 2025
c40d23b
Reverting to .HasCount because apparently the MSTest analyzers requir…
JosephPotapenko Oct 2, 2025
75e8527
Making the checking correct answer and the quitting functionalities i…
JosephPotapenko Oct 2, 2025
8dffbfa
Previously, the "type quit to exit message" displayed at the very end…
JosephPotapenko Oct 2, 2025
cdb3d52
renames tests as suggested
chanderlud Oct 2, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 28 additions & 2 deletions PrincessBrideTrivia/PrincessBrideTrivia.Tests/ProgramTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ public void LoadQuestions_RetrievesQuestionsFromFile()
public void DisplayResult_ReturnsTrueIfCorrect(string userGuess, bool expectedResult)
{
// Arrange
Question question = new();
question.CorrectAnswerIndex = "1";
Question question = new() { CorrectAnswerIndex = "1" };

// Act
bool displayResult = Program.DisplayResult(userGuess, question);
Expand Down Expand Up @@ -69,6 +68,33 @@ public void GetPercentCorrect_ReturnsExpectedPercentage(int numberOfCorrectGuess
Assert.AreEqual(expectedString, percentage);
}

[TestMethod]
public async Task GenerateQuestions_Success()
{
var apiKey = Environment.GetEnvironmentVariable("OPENAI_API_KEY");

if (apiKey == null)
{
return; // test cannot run without the API key
}

Question q = await TriviaGenerator.GeneratePrincessBrideQuestionAsync(apiKey);
Assert.IsNotNull(q);
}
Comment on lines +71 to +83

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably haven't gotten to the point in class that we could test this easier, but relying on an external API for testing and CI/CD will make the test flaky (sometimes pass, sometimes fail, sometimes won't even test functionality - as it does here, and ultimately passes because no exceptions are thrown).

If you want to look ahead, Moq is a great way to mock implementations to insert "mocked" implementation for testing purposes. It's used mostly on interfaces and abstract classes, so you'd have to rearrange your architecture.


[TestMethod]
public async Task GenerateQuestions_GeneratesFourChoices_Success()
{
var apiKey = Environment.GetEnvironmentVariable("OPENAI_API_KEY");

if (apiKey == null)
{
return; // test cannot run without the API key
}

Question q = await TriviaGenerator.GeneratePrincessBrideQuestionAsync(apiKey);
Assert.HasCount(4, q.Answers);
}

private static void GenerateQuestionsFile(string filePath, int numberOfQuestions)
{
Expand Down
3 changes: 1 addition & 2 deletions PrincessBrideTrivia/PrincessBrideTrivia.sln
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@

Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio Version 17
VisualStudioVersion = 16.0.29215.179
VisualStudioVersion = 17.14.36518.9 d17.14
MinimumVisualStudioVersion = 10.0.40219.1

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should be here. Be sure to review all changes before staging/committing.

Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "PrincessBrideTrivia", "PrincessBrideTrivia\PrincessBrideTrivia.csproj", "{43E8309C-D064-4743-B038-16F808E98D98}"
EndProject
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
<ImplicitUsings>enable</ImplicitUsings>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="OpenAI" Version="2.5.0" />
</ItemGroup>

<ItemGroup>
<None Update="Trivia.txt">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
Expand Down
99 changes: 77 additions & 22 deletions PrincessBrideTrivia/PrincessBrideTrivia/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,34 +2,91 @@

public class Program
{
public static void Main(string[] args)
public static async Task Main()
{
var apiKey = Environment.GetEnvironmentVariable("OPENAI_API_KEY");

string filePath = GetFilePath();
Question[] questions = LoadQuestions(filePath);

int numberCorrect = 0;
int numberOfQuestions = questions.Length;
bool runWhile = true;

// ✅ Show quit option at the very start
Console.WriteLine("Type 'quit' anytime to exit");

for (int i = 0; i < questions.Length; i++)
{
bool result = AskQuestion(questions[i]);
(bool result, bool quitProgram) = AskQuestion(questions[i]);
if (quitProgram)
{
runWhile = false;
break;
}
if (result)
{
numberCorrect++;
}
}
Console.WriteLine("You got " + GetPercentCorrect(numberCorrect, questions.Length) + " correct");

// ✅ If user answered all file questions and we are NOT running AI loop, print score now

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments (especially, I assume, LLM comments 😉) that don't add clarity to the reader is against guidelines.

if (!runWhile || apiKey == null)
{
Console.WriteLine($"You got {GetPercentCorrect(numberCorrect, numberOfQuestions)} correct");
return;
}

while (runWhile)
{
try
{
Question q = await TriviaGenerator.GeneratePrincessBrideQuestionAsync(apiKey);

(bool result, bool quitProgram) = AskQuestion(q);

if (quitProgram)
{
runWhile = false;
}
else
{
numberOfQuestions++;
if (result)
{
numberCorrect++;
}
}
}
catch (InvalidOperationException ex)
{
Console.WriteLine(ex.Message);
continue;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a feeling if an exception was thrown here, it wouldn't be good to continue on. That'd signal there's something wrong with the OpenAI API or your TriviaGenerator code, and it probably wouldn't fix itself between iterations. Another dangerous thought is, if it was left to run by itself and kept running into this exception, it could run down your OpenAI tokens and increase your budget, since the exception can occur before ever asking for user input.

}
}

// ✅ Always print score at the very end
Console.WriteLine($"You got {GetPercentCorrect(numberCorrect, numberOfQuestions)} correct");
}

public static string GetPercentCorrect(int numberCorrectAnswers, int numberOfQuestions)
{
return (numberCorrectAnswers / numberOfQuestions * 100) + "%";
double roundedPercent = Math.Round((float)numberCorrectAnswers / (float)numberOfQuestions * 100, 2);
return $"{roundedPercent}%";
}

public static bool AskQuestion(Question question)
public static (bool answeredCorrectly, bool quitProgram) AskQuestion(Question question)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd recommend against returning Tuples, as I think Mark said on Thursday. Could use an out parameter or maybe restructure logic around determining quitting.

{
DisplayQuestion(question);

string userGuess = GetGuessFromUser();
return DisplayResult(userGuess, question);

if (userGuess == "quit")
{
return (false, true);
}

bool isCorrect = DisplayResult(userGuess, question);
return (isCorrect, false);
}

public static string GetGuessFromUser()
Expand All @@ -51,10 +108,10 @@ public static bool DisplayResult(string userGuess, Question question)

public static void DisplayQuestion(Question question)
{
Console.WriteLine("Question: " + question.Text);
Console.WriteLine($"Question: {question.Text}");
for (int i = 0; i < question.Answers.Length; i++)
{
Console.WriteLine((i + 1) + ": " + question.Answers[i]);
Console.WriteLine($"{i + 1}: {question.Answers[i]}");
}
}

Expand All @@ -71,22 +128,20 @@ public static Question[] LoadQuestions(string filePath)
for (int i = 0; i < questions.Length; i++)
{
int lineIndex = i * 5;
string questionText = lines[lineIndex];

string answer1 = lines[lineIndex + 1];
string answer2 = lines[lineIndex + 2];
string answer3 = lines[lineIndex + 3];

string correctAnswerIndex = lines[lineIndex + 4];
Question question = new()
{
Text = lines[lineIndex],
Answers = new string[3],
CorrectAnswerIndex = lines[lineIndex + 4]
};
question.Answers[0] = lines[lineIndex + 1];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

question.Answers[1] = lines[lineIndex + 2];
question.Answers[2] = lines[lineIndex + 3];

Question question = new();
question.Text = questionText;
question.Answers = new string[3];
question.Answers[0] = answer1;
question.Answers[1] = answer2;
question.Answers[2] = answer3;
question.CorrectAnswerIndex = correctAnswerIndex;
questions[i] = question;
}

return questions;
}
}
94 changes: 94 additions & 0 deletions PrincessBrideTrivia/PrincessBrideTrivia/TriviaGenerator.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
using OpenAI.Chat;
using System.Text.Json;
using System.Text.Json.Serialization;

namespace PrincessBrideTrivia
{
public static class TriviaGenerator
{
private const string Model = "gpt-4.1";

/// <summary>
/// Generates one Princess Bride multiple-choice question using the OpenAI API.
/// </summary>
public static async Task<Question> GeneratePrincessBrideQuestionAsync(string apiKey, int choices = 4)
{
if (string.IsNullOrWhiteSpace(apiKey))
throw new ArgumentException("API key is required.", nameof(apiKey));

if (choices is < 3 or > 6)
Copy link

@BillMillerCoding BillMillerCoding Oct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good discussion you two :) This is definitely what we're looking for here. Respectful discussion and explanation of your reasoning.

Side note: I would tend to agree with William here, though. I think it'd be best practice to set them in local or global fields, for future extensibility, like William said. Always sticking to best practice builds up good habits.

throw new ArgumentOutOfRangeException(nameof(choices), "choices must be between 3 and 6.");

var client = new ChatClient(model: Model, apiKey: apiKey);

var system = """
You generate trivia strictly about the 1987 film "The Princess Bride".
Output ONLY a single JSON object with this exact C#-friendly shape:
{
"Text": string, // the question text
"Answers": string[], // exactly N distinct options, concise, no markup
"CorrectAnswerIndex": string // "1"-based index of the correct answer, as a string
}
Rules:
- The question must be unambiguous and answerable from the film (not the novel).
- Answers must be short (max ~80 chars each) and mutually exclusive.
- Make sure to switch up which index is correct.
- Do not include explanations, hints, or extra keys.
- Do not include code fences. Print raw JSON only.
""";

var user = $"""
Create ONE multiple-choice question.
Number of options: {choices}.
Difficulty: hard.
""";

var completion = await client.CompleteChatAsync(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd lean away from using 'var' here. Mark's guidelines advise against it here: AVOID using implicitly typed local variables ( var ) unless the data type of the assigned value is obvious.

new ChatMessage[]
{
new SystemChatMessage(system),
new UserChatMessage(user)
},
new ChatCompletionOptions
{
// Mild creativity
Temperature = (float)0.7
});

var message = completion.Value.Content[0];
var json = message.Text?.Trim();

if (string.IsNullOrWhiteSpace(json))
throw new InvalidOperationException("Model returned empty content.");

// Strict JSON parse
var question = JsonSerializer.Deserialize<Question>(json, new JsonSerializerOptions

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, although if the reader has experience with JsonSerializer, they could determine the type. It's not immediately obvious though.

{
ReadCommentHandling = JsonCommentHandling.Disallow,
AllowTrailingCommas = false,
PropertyNameCaseInsensitive = true,
NumberHandling = JsonNumberHandling.Strict
});

Validate(question, choices);
return question!;
}

private static void Validate(Question q, int expectedChoices)
{
if (q is null) throw new InvalidOperationException("Failed to parse the model's JSON.");
if (string.IsNullOrWhiteSpace(q.Text)) throw new InvalidOperationException("Question text missing.");
if (q.Answers is null || q.Answers.Length != expectedChoices)
throw new InvalidOperationException($"Expected {expectedChoices} answers, got {q?.Answers?.Length ?? 0}.");

if (q.Answers.Any(string.IsNullOrWhiteSpace))
throw new InvalidOperationException("One or more answers are empty.");

if (!int.TryParse(q.CorrectAnswerIndex, out var idx))
throw new InvalidOperationException("CorrectAnswerIndex must be a numeric string.");

if (idx < 0 || idx >= q.Answers.Length)
throw new InvalidOperationException("CorrectAnswerIndex out of range.");
Comment on lines +79 to +91

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (q is null) throw new InvalidOperationException("Failed to parse the model's JSON.");
if (string.IsNullOrWhiteSpace(q.Text)) throw new InvalidOperationException("Question text missing.");
if (q.Answers is null || q.Answers.Length != expectedChoices)
throw new InvalidOperationException($"Expected {expectedChoices} answers, got {q?.Answers?.Length ?? 0}.");
if (q.Answers.Any(string.IsNullOrWhiteSpace))
throw new InvalidOperationException("One or more answers are empty.");
if (!int.TryParse(q.CorrectAnswerIndex, out var idx))
throw new InvalidOperationException("CorrectAnswerIndex must be a numeric string.");
if (idx < 0 || idx >= q.Answers.Length)
throw new InvalidOperationException("CorrectAnswerIndex out of range.");
if (q is null)
{
throw new InvalidOperationException("Failed to parse the model's JSON.");
}
if (string.IsNullOrWhiteSpace(q.Text))
{
throw new InvalidOperationException("Question text missing.");
}
if (q.Answers is null || q.Answers.Length != expectedChoices)
{
throw new InvalidOperationException($"Expected {expectedChoices} answers, got {q?.Answers?.Length ?? 0}.");
}
if (q.Answers.Any(string.IsNullOrWhiteSpace))
{
throw new InvalidOperationException("One or more answers are empty.");
}
if (!int.TryParse(q.CorrectAnswerIndex, out var idx))
{
throw new InvalidOperationException("CorrectAnswerIndex must be a numeric string.");
}
if (idx < 0 || idx >= q.Answers.Length)
{
throw new InvalidOperationException("CorrectAnswerIndex out of range.");
}

Might just be a personal thing, but it's not immediately easy to read through this block. Even though certain statements and blocks can be abbreviated, I'd stick towards making them clearer. Although it's a little more typing, I'd do the suggestion above. Or even having the first two if statements not be one line total.

Here's what Mark's guidelines say about it: AVOID omitting braces, except for the simplest of single-line if statements.

}
}
}