Assignment 6 #75
Assignment 6 #75
Conversation
…anderlud/EWU-CSCD371-2025-Fall into Assignment6-LambdaExpressions
…anderlud/EWU-CSCD371-2025-Fall into Assignment6-LambdaExpressions
There was a problem hiding this comment.
Pull Request Overview
This PR implements a console-based calculator application with support for basic arithmetic operations (+, -, *, /) using generic types with .NET 9.0. The implementation includes a shunting-yard algorithm to handle operator precedence and supports negative numbers.
Key changes:
- Created a generic
Calculator<TOperand>class with operator precedence support using the shunting-yard algorithm - Implemented a
Programclass with testableWriteLine/ReadLineproperties - Added comprehensive unit tests for calculator operations and program I/O
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updated checklist to mark all requirements as completed |
| Directory.Build.props | Added shared MSBuild properties for .NET 9.0, nullable reference types, and code analysis |
| Calculate/Program.cs | Implemented console program with testable I/O delegates |
| Calculate/Calculator.cs | Created generic calculator with operator precedence and comprehensive expression parsing |
| Calculate/Calculate.csproj | Added project file for console application |
| Calculate.sln | Created solution file with test project reference |
| Calculate.Tests/ProgramTests.cs | Added unit tests for Program I/O redirection |
| Calculate.Tests/MSTestSettings.cs | Configured parallel test execution |
| Calculate.Tests/CalculatorTests.cs | Added comprehensive calculator tests including edge cases |
| Calculate.Tests/Calculate.Tests.csproj | Added test project with MSTest configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Calculate/Program.cs
Outdated
| { | ||
| WriteLine("Goodbye"); | ||
| run = false; | ||
| } else if (calculator.TryCalculate(equation, out float result)) |
There was a problem hiding this comment.
The 'else' should be on a new line to maintain consistent formatting with the rest of the codebase. All other closing braces are followed by keywords on new lines.
| } else if (calculator.TryCalculate(equation, out float result)) | |
| } | |
| else if (calculator.TryCalculate(equation, out float result)) |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
|
I'm not really seeing much testing that covers program.cs. But technically, this does follow the requirements. |
|
Create a Console project called "Calculate.". ✔ Fundamentals Extra Credit not done in this repo |
|
The only thing I can imagine can be better is when you're dealing with exit being typed in, but say they accidentally type it in uppercase, and it tells them invalid equation, but it's not technically an equation. You should probably output "invalid input" in to begin with (if other random input is given). If you want, I believe you can ignore the case with: if (string.Equals(equation, "exit", StringComparison.OrdinalIgnoreCase)) It's kind of messy, but it would be basically the same thing you wrote while ignoring the case of the input. Not exactly necessary, but a little bit more user friendly. |
|
Create a Console project called "Calculate". ✔ |
Extra CreditDo one of the following two options (or both if you want extra, extra credit) :)
Fundamentals
|
|
Create a Console project called "Calculate.". ✔ Refactor the redirect portion of the Program class into 'ProgramBase` ✔ |
…anderlud/EWU-CSCD371-2025-Fall into Assignment6-LambdaExpressions
SummarySummary
CoverageCalculate - 72.6%
|
Joshua-Lester3
left a comment
Joshua-Lester3
left a comment
There was a problem hiding this comment.
Instructions
- Create a Console project called "Calculate". ✔
- Define a Program Class
- Define two init-only setter properties,
WriteLineandReadLine, that contain delegates for writing a line of text and reading a line of text respectively ✔ - Write a test that sets these properties at construction time and then invokes the properties and verifies the expected behavior occurs. ✔
- Set the default behavior for the
WriteLineandReadLineproperties to invokeSystem.Consoleversions of the methods and add an empty default constructor. ✔
- Define two init-only setter properties,
- Define a Calculator class ✔
- Define static
Add,Subtract,Multiply, andDividemethods that have two parameters and return a third parameter. ❌ Kind of nitpicky, but this says return a third parameter. So my assumption is there's a third out parameter. - Define a read-only property,
MathematicalOperations, of typeSystem.Collections.Generics.IReadOnlyDictionary<TKey,TValue>that:- is initialized to a
System.Collections.Generics.Dictionary<<TKey,TValue>instance that. ✔- Uses
charfor the key corresponding to the operators +, -, *, and /. ✔ - Has values that correspond with the Add, Subtract, Multiply, and Divide methods. ✔
- Uses
- is initialized to a
- Implement a
TryCalculatemethod following "TryParse" pattern ✔- Valid
calculationexpressions include such strings as "3 + 4", "42 - 2", etc. ✔ - If there is no whitespace around the operator, you can assume the
calculationis invalid and return false. Similarly if the operands are not integers. ✔ - Use
string.Split(), pattern matching, logical and operators to parse the string in their entirety ✔ - Index into the
MathematicalOperationsmethod using the operator parsed during pattern matching to find the corresponding implementation and invoke it. ✔
- Valid
- Define static
- Implement the Program class to instantiate the calculator and invoke it based on user input from the console. ✔
- Be sure to use the
WriteLine/ReadLineproperties onProgramfor testing the input and output of your program. ❌ Program wasn't really tested (at least Main wasn't). I'm assuming these properties were there to help test Program.Main.
Extra Credit
Do one of the following two options (or both if you want extra, extra credit) :)
- Refactor the redirect portion of the
Programclass into 'ProgramBase` ❌- Move ProgramBase into a ConsoleUtilities assembly to be used in other console-based projects
- Use generics the mathematical operations methods and consider using generic constraints (requires .NET 7.0) ❌
Fundamentals
- Place all shared project properties into a
Directory.Build.propsfile. - Place all shared project items into a
Directory.Build.targetsfile. (optional) - nullable reference types is enabled ✔
- Ensure that you turn on code analysis for all projects(EnableNETAnalyzers) ✔
- Set LangVersion and the TargetFramework to the latest released versions available (preview versions optional) ✔
- and enabled .NET analyzers for both projects ✔
- For this assignment, always use
Assert.AreEqual<T>()(the generic version) ✔ - All of the above should be unit tested ❌ I'd say it's not fully tested, especially looking at the code coverage report on the PR.
- Choose simplicity over complexity ✔
|
|
||
| <PropertyGroup> | ||
| <OutputType>Exe</OutputType> | ||
| <TargetFramework>net9.0</TargetFramework> |
There was a problem hiding this comment.
Since this is in the props file, I'd remove it (targetframework) here. If you update props in the future, you'd have to change this as well, since it'd overwrite it.
| ['/'] = Divide | ||
| }; | ||
|
|
||
| public static int Add(int a, int b) => a + b; |
There was a problem hiding this comment.
If we're following requirements very specifically, this would probably look like
public static void Add(int a, int b, out int result) ....|
|
||
| public static int Divide(int a, int b) => a / b; | ||
|
|
||
| public bool TryCalculate(string calculation, out int result) |
There was a problem hiding this comment.
This is pretty cool. A little complex and maybe could be refactored a bit because it's a long function. I don't think the requirements asks for this level of complexity (as it only specifies simple examples like "4 + 4", etc.) - though that's not a bad thing.
| Assert.AreEqual<int>(5, ops['+'](2, 3)); | ||
| Assert.AreEqual<int>(-1, ops['-'](2, 3)); | ||
| Assert.AreEqual<int>(6, ops['*'](2, 3)); | ||
| Assert.AreEqual<int>(2, ops['/'](6, 3)); |
There was a problem hiding this comment.
I'd probably separate these into multiple tests - there's quite a few ways for this test to fail. It should usually only have one reason to fail.
| } | ||
|
|
||
| [TestMethod] | ||
| public void BasicArithmeticMethods_ValidIntInputs_ExpectedResults() |
There was a problem hiding this comment.
Same here as above.
| Calculator calculator = new(); | ||
| bool run = true; | ||
|
|
||
| while (run) |
There was a problem hiding this comment.
Consider that having an infinite loop creates a certain kind of interactive experience like a REPL vs if you did not have the loop the app would exit on each invocation and could be used like in an automated tool call fashion.
| int a = evaluationStack.Pop(); | ||
|
|
||
| // divide by 0 is not allowed | ||
| if (token[0] == '/' && b == 0) |
There was a problem hiding this comment.
Consider using the MathematicalOperations division key instead of hard coding this here.
| evaluationStack.Push(number); | ||
| } | ||
| else if (token.Length == 1 && MathematicalOperations.TryGetValue(token[0], out var op)) | ||
| { |
There was a problem hiding this comment.
In the instructions it mentions to "Index into the MathematicalOperations method using the operator parsed during pattern matching to find the corresponding implementation and invoke it."
9 participants
I worked on this project with @ulises-aguilar