-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unit tests for IDatumConverter implementations #140
Conversation
Hey @tetious, this all looks good... except the DatumHelper extensions. I'm really not a fan of these extension methods for three reasons; (a) the pollute the global namespace by adding extension methods on primitives, they'd constantly be showing up in intellisense for library consumers, (b) they're useless to consumers of the library because they expose Datums, which aren't a type the consumer would use, and (c) they don't have all the same logic checks that the primitive datum converters have, so they're inconsistent and in the case of the long type potentially dangerous. |
Yeah, that makes sense. Would you object if I instead made them internal to the .Test assembly? Or would that still be annoying? If so, I'll just make them static helpers in the test assembly. |
Internal to the Test assembly sounds good to me. :-) |
Done. :) |
Seems like teamcity.codebetter.com is offline right now, so I ran the updated test suite myself. Looks good. Merging. Thanks Greg. :-) |
Unit tests for IDatumConverter implementations, fixes #60
This should fix #60.
Please let me know if you think anything should be added/changed.
Also, I added some Datum generation extension methods to reduce repetition. (DatumHelper)