OutpostUniverse / OP2Utility

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

Add DynamicMemoryWriter class #263

Closed DanRStevens closed 5 years ago

DanRStevens commented 5 years ago

I'm still working on the test code for this one, but I thought I'd get the code up early for review purposes.

I was at first thinking of just making this a Writer, then thought a ForwardWriter would also be easy, and slightly more useful. Then I thought why not make it BidirectionalWriter, though I took one slight shortcut in the implementation, where seeking sets the length of the stream (either by truncating, or 0 filling).


This writes to a dynamically allocated and resizable memory buffer. After writing, the GetReader() method will return a MemoryReader object with a view of the memory.

This class can be used to aid writing test code. Writing can be done in-memory, and then verified with the associated MemoryReader object. There is no need to write to a slow disk, nor any need to clean up temporary files. This makes writing test code simpler and faster.

DanRStevens commented 5 years ago

Ok, I've added the tests I wanted to add. I also updated a few existing test cases to use the DynamicMemoryWriter. Two cases for update were: 1) Using a MemoryWriter with a statically sized buffer. 2) Using a FileWriter, and then deleting the file afterwards. (But not explicitly testing a filename path)

I think this is ready to merge now.

Brett208 commented 5 years ago

Should have time to review later today or tomorrow. Sorry for the excessive delay. Been trying to focus on NetFix/OP2Internal.

DanRStevens commented 5 years ago

Thanks for the updates. Just realized that although the AppVeyor build appeared to have passed for the code I'd pushed, it never actually ran the new tests since they hadn't been added to the test project file yet.


I've made some slight updates to the static_cast code to use a type consistent with the range check. It's possible for a standard container to use a type other than std::size_t, so they have their own size_type type alias, though it's usually just an alias for std::size_t.

I was tempted to update the type in the DynamicMemoryWriter constructor as well. The preallocateSize parameter still uses std::size_t. We could try to DRY things up a bit more, though not without moving the private member variable ahead of the constructor, which I didn't want to do. Probably time to move on.

Brett208 commented 5 years ago

Thanks for the documentation on Dynamic Memory Writer. Good point on size_type.