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

Implement a clean facade API #19

Merged
merged 2 commits into from
May 22, 2021
Merged

Conversation

manxorist
Copy link
Contributor

Fixes #18.

Buffer::OutOfMemoryError has exactly the same semantics as standard C++ std::bad_alloc. ancient can already throw std::bad_alloc (it is thrown by e.g. std::make_unique) and thus client code already has to catch std::bad_alloc if it wants to deal with out-of-memory situations anyway. Splitting out-of-memory condition into 2 different exceptions only complicates client code for no obvious gain.
 *  Add a new single public API header <ancient/ancient.hpp>.
 *  Use inline namespace APIv2, just in case this ever needs to be reworked without breaking old ABI.
 *  Namespace all internal symbols into namespace ancient::internal (mainly to avoid name conflict between public Decompressor wrapper and internal Decompressor).
 *  Move Decompressor.hpp and Buffer.hpp back to their original locations.
 *  Move exception classes out of public Decompressor class (retain internal aliases in order to reduce diff noise).
 *  Completely shield external API from internal Decompressor API or ABI changes by using a pimpl-idiom with a publicly opaque type.
 *  In the public interface, do not expose a static create function, but instead handle construction in the constructor because the only failure case for create() is an exception anyway. We can do that externally now, as the externally visible Decompressor is not a polymorphic type anymore.
 *  Add a WrappedVectorBuffer that implements a Buffer interface around a reference to std::vector<uint8_t>.
 *  Avoid exposing any internal Buffer class and only use std::vector<uint8_t> or (const uint8_t*,size_t) tuples in public API.
 *  Add a new and simpler Decompressor::decompress() that handles allocation internally and returns the data instead of filling a pre-allocated out parameter.
    This is efficient since C++11 even in the case where the compiler does not do named return value optimization because C++11 specifies that returned data is moved if possible, and std::vector<uint8_t> is moveable.
 *  Remove old Decompressor::decompress() from public interface.
 *  Use std::optional<size_t> for getPackedSize(), getRawSize(), getImageSize(), and getImageOffset() in order to allow distinguishing actual 0 from "information not available/applicable".
 *  Rewrite main.cpp to new API.
@temisu temisu merged commit d99c9c2 into temisu:master May 22, 2021
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 this pull request may close these issues.

A cleaner public API
2 participants