search

mfenniak / rethinkdb-net

A C# / .NET client driver for RethinkDB.
Other
247 stars 37 forks source link

Unit tests for IDatumConverter implementations #140

Closed tetious closed 11 years ago

tetious commented 11 years ago

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)

mfenniak commented 11 years ago

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.

tetious commented 11 years ago

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.

mfenniak commented 11 years ago

Internal to the Test assembly sounds good to me. :-)

tetious commented 11 years ago

Done. :)

mfenniak commented 11 years ago

Seems like teamcity.codebetter.com is offline right now, so I ran the updated test suite myself. Looks good. Merging. Thanks Greg. :-)