Simplify canonical data types #430
Conversation
|
Oh boy, this is a big one :) Honestly I think my only critique is that I want Convert out of the Exercises/ dir! Obviously it works and less code is required to generate some of the tests. I don't see anything that here that is a major red flag to me. |
There was a problem hiding this comment.
Great work @ErikSchierboom! One minor nit but I'll go ahead and merge as-is.
| @@ -20,38 +20,30 @@ protected override void UpdateCanonicalData(CanonicalData canonicalData) | |||
| { | |||
| // Process expected | |||
| if (IsComplexNumber(canonicalDataCase.Expected)) | |||
| { | |||
There was a problem hiding this comment.
@ErikSchierboom - between you, @jpreese, and I are style for when to use braces is clearly different... there are PRs where they're being added and removed:)
There was a problem hiding this comment.
Haha, true. I tend to not bother with these kind of changes since it does boil down to preferences, but sometimes you just cant resist. I love braces!
There was a problem hiding this comment.
@ErikSchierboom ok sounds good!
| @@ -62,55 +54,36 @@ protected override string RenderTestMethodBodyAssert(TestMethodBody testMethodBo | |||
|
|
|||
| private static string RenderComplexNumberAssert(TestMethodBody testMethodBody) | |||
| { | |||
| var template = "Assert.Equal({{ ExpectedParameter }}.Real(), {{ TestedValue }}.Real(), 15);\r\nAssert.Equal({{ ExpectedParameter }}.Imaginary(), {{ TestedValue }}.Imaginary(), 15);"; | |||
| const string template = "Assert.Equal({{ ExpectedParameter }}.Real(), {{ TestedValue }}.Real(), precision: 15);\r\nAssert.Equal({{ ExpectedParameter }}.Imaginary(), {{ TestedValue }}.Imaginary(), precision: 15);"; | |||
There was a problem hiding this comment.
Nice! This change definitely makes the output feel more natural and not like it was written by a program :)
|
|
||
| return TemplateRenderer.RenderInline(template, testMethodBody.AssertTemplateParameters); | ||
| } | ||
|
|
||
| protected override HashSet<string> AddAdditionalNamespaces() | ||
| protected override HashSet<string> AddAdditionalNamespaces() => new HashSet<string> |
There was a problem hiding this comment.
Minor: For consistency with other places in the code this should all be on one line
|
Great work, @ErikSchierboom 🎉 |
This PR is quite a big refactoring of the canonical data type handling. It contains lots of things: some use of
dynamic, theConstructorInputandInputproperties referring to thePropertiesvalues instead of being copies, and much more :) With these changes, many exercises could further be simplified. The only exercises I'm not completely happy with are thealphameticsandword-countexercises, which deal with dictionary conversions.