Initial implementations #26
Conversation
| Console.WriteLine(); | ||
| hasHeaderWritten = true; | ||
| } | ||
| CheckIfSameHeaders(res); |
There was a problem hiding this comment.
This is a very good addition for safety, but not a very good addition for speed. Since this is really a tool to support unit tests, I think this is great.
| for (int i = 0; i < res.GetResultItems().Length; i++) | ||
| { | ||
| Console.Write(res.GetResultItems()[i].ResultName); | ||
| if (i < res.GetResultItems().Length - 1) |
There was a problem hiding this comment.
an alternative pattern is to write the first row, loop for i=1 to < length. for each i'th option write , value. that can eliminate the if check on each element.
Consequences/Results/Result.cs
Outdated
| @@ -8,6 +8,11 @@ public Result(ResultItem[] resultItems) | |||
| _resultItems = resultItems; | |||
| } | |||
|
|
|||
| public ResultItem[] GetResultItems() | |||
There was a problem hiding this comment.
a nice thing about unit tests is that you implement an activity that seeks to reach a goal (like writing results) and you find that your interfaces or implementations are missing things. That helps you ensure your code does what it needs to do. Great find and fix.
| [Fact] | ||
| public void TestHeaders() | ||
| { | ||
| var stringWriter = new StringWriter(); |
There was a problem hiding this comment.
great addition in a unit test to verify behavior with an assert.
Another way you could do this is to write a "stringbuilderresultswriter" where the result is internally stored in a string builder. I like your approach because it can be utilized in many ways.
ConsequencesTest/StructureTest.cs
Outdated
| public class StructureTest | ||
| { | ||
| [Fact] | ||
| public void TestSimpleDepth() |
There was a problem hiding this comment.
this is a great first test. write a test to write to the results writer too
…k on console output from StructureTest (test had weird behavior, console output passes intermittently but values are correct) #18
|
Testing console output is painful and poorly supported. I suggested to Jack, for the ConsoleWriter, to move the business logic of creating the string into a separate method, and honor the Write(Result res) interface with a very simple implementatin that calls the business logic, then writes the output string to console. I think that cleans up the testing and still accomplishes the goal. |
…console output
|
Looks like using the Issue number only soft links the commit to the issue, but doesn't close it out once merged. Moving forward, lets use Fix #xx as the syntax for commits that address specific issues. When we miss them, which is no big deal, add them manually from the PR page under "Development" I've added some of them already. |
| @@ -7,15 +7,18 @@ | |||
| namespace USACE.HEC.Geography; | |||
| public class BoundingBox | |||
| { | |||
| private Location _upperLeft; | |||
| private Location _lowerRight; | |||
| public Location _upperLeft; | |||
| @@ -1,8 +1,8 @@ | |||
| namespace USACE.HEC.Geography; | |||
| public class BoundingBox | |||
| { | |||
| public Location _upperLeft; | |||
| public Location _lowerRight; | |||
| private Location _upperLeft; | |||
…ate types to double to match NSI #27
No description provided.