OutpostUniverse / OP2Utility

C++ library for working with Outpost 2 related files and tasks.
MIT License
4 stars 0 forks source link

Add guard to vector write in Stream code #280

Closed DanRStevens closed 5 years ago

DanRStevens commented 5 years ago

This fixes #276.

Trying to pass a vector of non-trivially-copyable types will now result in a compile time error.

DanRStevens commented 5 years ago

Having unit tests to exercise the code being edited would be good.

In terms of this change, you can't really write a unit test for it, since the addition is a compile time check. If you failed the check, the code wouldn't compile. In particular, if unit tests attempted to fail this check, the unit test code wouldn't compile.

I suppose there are template based SFINAE ways of checking that certain code doesn't compile, so maybe it can be done, but it would require extra support code and it won't be pretty. It would also likely take days of research and testing. If we really want to do this, we should probably open an issue and defer it.

Additionally, Stream::Writer is an abstract class, so you could only test this change through a derived class. Perhaps DynamicMemoryWriter would be a good candidate to test from.

We should at least make sure the code is exercised for the happy path. The happy path being serializing of trivial data types. That's likely already covered indirectly in other unit tests. We should probably have a direct unit test though, as that would provide better error messages if something went wrong.

DanRStevens commented 5 years ago

I was thinking about the SFINAE check that something wouldn't compile, and realized that would be silly, since some of the other open changes are about ways to support serializing vectors of non-trivial types. Hence, code will compile and run once those changes are complete.

For now I've added some test code that exercises the happy path.