Assignment1 #20

BraydenMcMahon · 2025-10-01T00:50:07Z

Didnt have a partner since working on a teams problem and Benjamin said that was ok in class today just for this one.

nicoduvekot

This shows only Program.cs was updated, I would suggest adding unit tests for all your changes as well, to ensure these method changes are doing what you expect. I do like the changes you made though

Assignment Details:
Pull Request title includes "Assignment 1" in the title ✔
Partner has also made a commit ✔ (technically not, but instructor approval is stated)
Pull Request targets Assignment 1 branch ✔
Issue 1: Application no longer crashes ✔
Issue 2: Unit test properly passes ⚠️ the originals do, but you added functionality that is not being tested
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 ❌

thigiang16

I learned from how you fixed the code and used some of your improvements to enhance my own assignment. One thing I noticed: I don’t see any tests for the new changes (like parsing input, GetGuessIndexFromUser, etc.). Make sure to add some unit tests to cover these.

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 ❌

Al3xramirez · 2025-10-01T06:06:56Z

First off great job man! I love how you went back and improved the some of the starter code with better syntax, something I'm definitely gonna go back and do for my projects revisions.

With your new implementations, like the others have stated, it would be good practice to test those methods.
Also, when checking if the trivia file contains the five lines per question block, It could be possible that the file has 5 empty lines with nothing being contained in them. Thus passing empty lines that would pass.

Great Job!

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 ❌

JosephPotapenko · 2025-10-02T03:20:11Z

If the trivia file isn’t there, the program will crash with an unclear error. By checking it before that happens, you can let the user know what the issue is.

if (!File.Exists(filePath))
{
    Console.WriteLine($"Trivia file not found: {filePath}");
    return;
}

if (questions.Length == 0)
{
    Console.WriteLine("No questions found in the trivia file.");
    return;
}

All in all, it's just dealing with edge cases and isn't specifically necessary for the code in the form that it is right now. If you don't find it necessary- feel free to ignore the advice.

JosephPotapenko · 2025-10-02T03:26:09Z

Right now the code uses a for loop, but since there isn't really any reason to set the specific index and stop at a certain point, foreach might be better. It'll be simpler and cleaner as well.

int numberCorrect = 0;
foreach (var question in questions)
{
    if (AskQuestion(question))
    {
        numberCorrect++;
    }
}

JosephPotapenko · 2025-10-02T03:31:44Z

Towards the end of the Program.cs file, there are some unnecessary blank lines. Though this isn't a big deal, it would probably look better without them. Otherwise, the code looks clean, well maintained, and anything else that could be brought up regarding the code would be related to preference, or edge casing which wouldn't even be necessary in this specific program. Looks like good work to me!

KorbinWeiler · 2025-10-02T04:54:23Z

PrincessBrideTrivia/PrincessBrideTrivia/Program.cs

+
+            string answer1 = lines[lineIndex + 1].Trim();
+            string answer2 = lines[lineIndex + 2].Trim();
+            string answer3 = lines[lineIndex + 3].Trim();


You're doing the same operation 3 times here, this could be done better with a for loop which would cut down on repetition and make it easier to add more answer options in the future. This change would also require updating how you assign the answers to your question object.

Copilot

Pull Request Overview

This PR refactors a Princess Bride trivia game application with improvements to user input handling, string formatting, and code organization. The changes focus on making the code more robust and maintainable.

Improved string formatting by replacing concatenation with string interpolation
Enhanced input validation and error handling for user guesses and question data
Added better parsing logic with type safety and data validation

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-02T05:17:53Z

PrincessBrideTrivia/PrincessBrideTrivia/Program.cs

+    public static bool DisplayResult(string userGuess, Question question)
+{
+
+    if (!int.TryParse((userGuess ?? string.Empty).Trim(), out int guess1Based))
+    {
+        Console.WriteLine("Please enter 1, 2, or 3:");
+        return false;
+    }
+    return DisplayResult(guess1Based, question);
+}
+
+


This method creates a duplicate overload that's only used internally and adds confusion. The validation logic should be moved to GetGuessIndexFromUser() which already handles this properly, and this overload should be removed to avoid code duplication.

Suggested change

public static bool DisplayResult(string userGuess, Question question)

{

if (!int.TryParse((userGuess ?? string.Empty).Trim(), out int guess1Based))

{

Console.WriteLine("Please enter 1, 2, or 3:");

return false;

}

return DisplayResult(guess1Based, question);

}

Copilot · 2025-10-02T05:17:53Z

PrincessBrideTrivia/PrincessBrideTrivia/Program.cs

+        if (!int.TryParse(question.CorrectAnswerIndex?.Trim(), out int correct1Based))
+        {
+            Console.WriteLine("Question data invalid.");
+            return false;
+        }


The validation of question data should occur during loading in LoadQuestions() rather than during each question display. This would fail fast on invalid data and avoid repeated validation during gameplay.

Copilot · 2025-10-02T05:17:53Z

PrincessBrideTrivia/PrincessBrideTrivia/Program.cs

+
+
+
+


[nitpick] Remove the excessive blank lines before the return statement. This violates common C# formatting conventions and reduces code readability.

Suggested change

kellanburns · 2025-10-02T06:07:33Z

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 ❌

It looks like most of the formatting issues have already been addressed by other reviews, and still nothing bad for a one-day turnaround. The added feature is also a nice touch, but as observed, test-driven development makes writing the code first a bit like putting the chicken before the egg (so to speak).

github-actions · 2025-10-03T00:29:14Z

Summary


Generated on:	10/03/2025 - 00:29:10
Coverage date:	10/03/2025 - 00:29:08
Parser:	MultiReport (2x Cobertura)
Assemblies:	1
Classes:	2
Files:	2
Line coverage:	51% (48 of 94)
Covered lines:	48
Uncovered lines:	46
Coverable lines:	94
Total lines:	155
Branch coverage:	36.6% (11 of 30)
Covered branches:	11
Total branches:	30
Method coverage:	Feature is only available for sponsors
Tag:	44_18209265354

Coverage

PrincessBrideTrivia - 51%

Name	Line	Branch
PrincessBrideTrivia	51%	36.6%
PrincessBrideTrivia.Program	49.4%	36.6%
PrincessBrideTrivia.Question	100%

BraydenMcMahon · 2025-10-03T00:30:05Z

Thanks for the notes guys I have a very bad habit of never writing tests even though I know the importance of it so wrote a couple tests to cover the things I added such as making sure not to crash on dividing by zero and checking the override function as well.

Hellothereyoko · 2025-10-03T01:03:45Z

If the trivia file isn’t there, the program will crash with an unclear error. By checking it before that happens, you can let the user know what the issue is.
if (!File.Exists(filePath))
{
    Console.WriteLine($"Trivia file not found: {filePath}");
    return;
}

if (questions.Length == 0)
{
    Console.WriteLine("No questions found in the trivia file.");
    return;
}
All in all, it's just dealing with edge cases and isn't specifically necessary for the code in the form that it is right now. If you don't find it necessary- feel free to ignore the advice.

Thank you for the helpful advice! I'll make sure to be careful

Joshua-Lester3

Assignment Details

Pull Request title includes "Assignment 1" in the title ✔
Partner has also made a commit ✔❌ not necessary with one partner
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 ✔

Joshua-Lester3 · 2025-10-07T00:34:11Z

PrincessBrideTrivia/PrincessBrideTrivia.Tests/ProgramTests.cs

+[TestMethod]
+public void DisplayResult_IntOverload_ReturnsTrueWhenGuessMatches()
+{
+
+    var q = new Question { CorrectAnswerIndex = "3" };
+    bool result = Program.DisplayResult(3, q);
+    Assert.IsTrue(result);
+}
+
+[TestMethod]
+public void DisplayResult_IntOverload_ReturnsFalseWhenGuessDoesNotMatch()
+{
+    var q = new Question { CorrectAnswerIndex = "2" };
+    bool result = Program.DisplayResult(1, q);
+    Assert.IsFalse(result);
+}


It looks like formatting/indentation is off here. I see a few other places as well. Maybe look into auto formatting or the format shortcut for Visual Studio or VS Code or whatever editor you use :)

Joshua-Lester3 · 2025-10-07T00:34:48Z

PrincessBrideTrivia/PrincessBrideTrivia/Program.cs

+
+
+    public static bool DisplayResult(string userGuess, Question question)
+{


Ex: Same weird formatting here. This'll be the last reference I make, though I see more.

Joshua-Lester3 · 2025-10-07T00:36:50Z

PrincessBrideTrivia/PrincessBrideTrivia/Program.cs

-            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();
-            question.Text = questionText;
-            question.Answers = new string[3];
-            question.Answers[0] = answer1;
-            question.Answers[1] = answer2;
-            question.Answers[2] = answer3;
-            question.CorrectAnswerIndex = correctAnswerIndex;
+            int lineIndex = i * block;
+
+            string questionText = lines[lineIndex].Trim();
+
+            string answer1 = lines[lineIndex + 1].Trim();
+            string answer2 = lines[lineIndex + 2].Trim();
+            string answer3 = lines[lineIndex + 3].Trim();
+
+            string correctAnswerIndex = lines[lineIndex + 4].Trim();
+
+            Question question = new()
+            {
+                Text = questionText,
+                Answers = new[] { answer1, answer2, answer3 },
+                // store the raw 1-based index string; we parse later
+                CorrectAnswerIndex = correctAnswerIndex
+            };
+
+
+            questions[i] = question;


I like the refactoring here. Definitely makes sense, but also agree with Korbin on possibly doing a for loop for the three answers. It's about the same amount of work, but it'd be cleaner IMO.

Joshua-Lester3 · 2025-10-07T00:39:03Z

PrincessBrideTrivia/PrincessBrideTrivia/Program.cs

-        Question[] questions = new Question[lines.Length / 5];
+        // Each question block is 5 lines: Q, A1, A2, A3, CorrectIndex
+        int block = 5;
+        if (lines.Length % block != 0)


I'm curious what would happen if there was an extra newline. Might break this.

Joshua-Lester3 · 2025-10-07T00:42:12Z

PrincessBrideTrivia/PrincessBrideTrivia/Program.cs

+        }
+    }
+
+    public static bool DisplayResult(int userGuessIndex1Based, Question question)


I probably would use "One" here instead of "1". I'm not sure there's a guideline against having numbers in variable names (@BenjaminMichaelis correct me if I'm wrong), but it'd be clearer to swap to "One". It took me a couple reads to see that it was a "1" and not a lower-case "L" or upper-case "i".

Joshua-Lester3 · 2025-10-07T00:44:54Z

@BraydenMcMahon make sure to respond to/resolve all peer review comments - including Copilot's review.

bmcmahon4 added 2 commits September 30, 2025 10:56

made changes to quiz

016a12c

fixing small template things

1c5b773

nicoduvekot suggested changes Oct 1, 2025

View reviewed changes

thigiang16 reviewed Oct 1, 2025

View reviewed changes

KorbinWeiler reviewed Oct 2, 2025

View reviewed changes

BenjaminMichaelis requested a review from Copilot October 2, 2025 05:17

Copilot AI reviewed Oct 2, 2025

View reviewed changes

Added new tests per peer review notes from students

3ba475d

Joshua-Lester3 reviewed Oct 7, 2025

View reviewed changes



		public static bool DisplayResult(string userGuess, Question question)
		{

Assignment1 #20

Are you sure you want to change the base?

Assignment1 #20

Conversation

BraydenMcMahon commented Oct 1, 2025

Uh oh!

nicoduvekot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thigiang16 left a comment

Choose a reason for hiding this comment

Uh oh!

Al3xramirez commented Oct 1, 2025

Uh oh!

JosephPotapenko commented Oct 2, 2025

If the trivia file isn’t there, the program will crash with an unclear error. By checking it before that happens, you can let the user know what the issue is.

All in all, it's just dealing with edge cases and isn't specifically necessary for the code in the form that it is right now. If you don't find it necessary- feel free to ignore the advice.

Uh oh!

JosephPotapenko commented Oct 2, 2025

Right now the code uses a for loop, but since there isn't really any reason to set the specific index and stop at a certain point, foreach might be better. It'll be simpler and cleaner as well.

Uh oh!

JosephPotapenko commented Oct 2, 2025

Uh oh!

KorbinWeiler Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

kellanburns commented Oct 2, 2025

Uh oh!

github-actions bot commented Oct 3, 2025

Summary

Coverage

Uh oh!

BraydenMcMahon commented Oct 3, 2025

Uh oh!

Hellothereyoko commented Oct 3, 2025

If the trivia file isn’t there, the program will crash with an unclear error. By checking it before that happens, you can let the user know what the issue is.

All in all, it's just dealing with edge cases and isn't specifically necessary for the code in the form that it is right now. If you don't find it necessary- feel free to ignore the advice.

Uh oh!

Joshua-Lester3 left a comment

Choose a reason for hiding this comment

Assignment Details

Extra Credit

Uh oh!

Joshua-Lester3 Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

Joshua-Lester3 Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

Joshua-Lester3 Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

Joshua-Lester3 Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

Joshua-Lester3 Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

Joshua-Lester3 commented Oct 7, 2025

Uh oh!

Uh oh!

nicoduvekot left a comment •

edited

Loading