Re: [boost] [AFIO] Review (or lack of it)

Subject: Re: [boost] [AFIO] Review (or lack of it)
From: Hartmut Kaiser (hartmut.kaiser_at_[hidden])
Date: 2015-08-30 11:19:12

> I did a bit of mocking up of what the refactored data_store interface
> for that AFIO tutorial will look like:
> 
> https://gist.github.com/ned14/163ac57c937bda61e5c9
> 
> Ok, so it's no longer the world's simplest key-value store API and I
> deliberately dropped the STL iostreams interop, but the big
> difference is that this one is real-world ready and probably quite
> close to a final API design for such a thing.

*Looks* much better, you seem to listen after all. I wouldn't call that to
be real-world-ready without a thorough specification of the semantics,
though. 

For instance:

- What happens if commit() was called on your transaction, the returned
future is not ready yet, but the ~transaction() is called? 
- Why is your transaction object not RAII? 
- Why do you need begin_transaction? 
- Why do you need two load_blob() functions? 
- Why is your buffertype so over-specified, shouldn't any range do? 
- Why can't the user specify/provide the type of the hash? std::hash<>
anybody?
- Why does one of the load_blob returns a
future<std::shared_ptr<buffers_type>>? Handle to handle to handle to data
this time? Shared ownership again, really?
- Can this blob-store be distributed over many machines? Can it be in memory
only (with transparent disk backup if needed)?
- Why do you specify undefined behavior into your API (see your union). Why
do you need this in the first place?
- What's the concurrency/thread-safety guarantees of each of the functions?

etc. many more open questions remain. Please, don't get me wrong, I don't
need/want answers to those questions right away, I'm just listing those for
you to think about.

I however suggest that you redesign AFIO first (including the proposed AFIO
API), before you even start thinking about submitting this to Boost review. 

Regards Hartmut
---------------
http://boost-spirit.com
http://stellar.cct.lsu.edu