Skip to content
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

Add generic stream interface for serialization / deserialization #596

Open
kiplingw opened this issue Oct 29, 2024 · 0 comments · May be fixed by #598
Open

Add generic stream interface for serialization / deserialization #596

kiplingw opened this issue Oct 29, 2024 · 0 comments · May be fixed by #598

Comments

@kiplingw
Copy link

Great work @yurymalkov. This is a simple suggestion for a design improvement that I might be able to implement when I have more time. For now I'm creating this issue as a discussion topic.

Currently serialization / deserialization of all internal state is performed via void saveIndex(const std::string &location) and void loadIndex(const std::string &location, SpaceInterface<dist_t> *s, size_t max_elements_i = 0) interfaces respectively. This works fine if the user is always comfortable with the state being preserved as a standalone uncompressed file on disk. But there are many scenarios where the user might want to store / retrieve this data within some other file that contains other unrelated data too. Or perhaps they don't want to store it on disk at all but would like to send it over the wire. Or perhaps they want to do some fancy cryptography with it. Or perhaps they want to compress / decompress it. The latter especially makes sense for very large datasets.

The solution to this is to have the aforementioned two interfaces wrap two new public ones that work with std::ostream and std::istream streams respectively instead of taking explicit paths to file system paths.

On a related note, the const std::string &location declarations should probably be migrated to const std::filesystem::path &location if or when the library is ready to be migrated to C++17. Although I understand that for the time being the intent is to maintain compatibility with C++11.

@kiplingw kiplingw linked a pull request Oct 31, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant