Repository navigation
hyperia-forex: Add missing test cases and fix Exemplar.cs - #1666
ErikSchierboom merged 6 commits into
Conversation
The existing test suite only covers the `==` operator, which is introduced in the initial sub-problem. All of the other operators were not tested.
The fact that the `*` operator is overloaded twice makes sense because multiplication is commutative (2 * X == X * 2), but the same does not hold for division. In fact, `2 / new CurrencyAmount(10, "UD")` does not even make sense, what type should it return? Therefore, I don't think this method should be part of the exemplar implementation at all.
The existing implementation does not match the specification and also results in a StackOverflow because it calls itself.
Yeah, let's remove this. |
|
I just realized that adding these test cases will mean that you are no longer able to run the tests half-way through the exercises, because the test suite won't compile until all the operators on |
|
@sanderploegsma Yeah this is a problem that we have with several exercises, and not something that is easy to fix. It's fine for now. Merging this is blocked by exercism/csharp-test-runner#67 BTW. I just found out that I don't unskip non- |
|
Ah that's a good catch! I did check the |
ErikSchierboom
left a comment
ErikSchierboom
left a comment
There was a problem hiding this comment.
Test runner update has been deployed. Thanks!
2 participants
This adds test cases to cover all tasks of the exercise, instead of just the
==operator. To get all of the tests to pass two changes were required to the exemplar implementation:!=operator was not correctly overridden: it actually calls itself causing a stack overflow./operator was overloaded to support bothnew CurrencyAmount(100, "USD") / 2mand2m / new CurrencyAmount(100, "USD"). The first version is part of the specification and makes total sense. However, the second version is not mentioned in the instructions anywhere, and in my opinion also doesn't make any sense: what would this even return?This fixes #1652.