search

OutpostUniverse / OP2Utility

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

Separate seek into seek forward and seek backward #244

Closed Brett208 closed 5 years ago

Brett208 commented 5 years ago

Should close #160

I didn't spend any time improving or updating the unit tests beyond changing them to bi-directional.

DanRStevens commented 5 years ago

Hmm, was just thinking "Bi" probably shouldn't be capitalized separately from "Bidirectional".

Brett208 commented 5 years ago

Agree that Bidirectional is probably a bit better than BiDirectional.

If you think there is value in continuing to fracture streams into a larger hierarchy, then I suppose we can. Since it is a public API, making the changes sooner could be a good call for stability in the future but I don't think we have identified an immediate use case for anything new. So we could be making changes that are never used or don't line up with a currently unknown use case.

-Brett

DanRStevens commented 5 years ago

I think you have a point, we should delay making the change until we have a need for it. Splitting a class interface like that should be a backwards compatible binary change, as long as the order of the methods doesn't change. It may introduce some source incompatibilities though due to the renaming.

Let's defer on further class splitting.

I'll leave the Bidirectional rename to you to avoid untested/inconsistent changes in the project files.

Brett208 commented 5 years ago

@DanRStevens, if this looks good to you, feel free to merge.

Thanks, Brett