On 28.08.2015 05:28, Niall Douglas wrote:
On 27 Aug 2015 at 12:54, Roland Bock wrote:
* "The /final/ named blob store design" starts with "Caution: This section was not finished". * "The performance of and problems with this third generation design" consists of "This section was not completed in time[...]" * So basically the crucial parts of the blob store sequence are incomplete at best.
The final example is over 1000 lines of code. I didn't think it mattered hugely to the review.
You mentioned this earlier as well. I keep wondering why an example requires more than 1000 lines of code? Is it because the task is too broad or because the library interface is too verbose? In both cases something should be done about it - either change the docs or the library.
f) Code (since the implementation is going to change anyway, this might all be useless):
* /attic: Seriously?
AFIO will be three years old soon. It had a life before being ported to Boost in 2013. There are still bits and pieces in the attic directory of use to me.
The implementation in Boost should preferably be free from lecacy code. If that code is generally useful then why is it not in the library? If it's only usefult to you then keep it in your own repo separate from the library.
* I am not a big fan of #if 0 in release code
For the examples, logged at https://github.com/BoostGSoC13/boost.afio/issues/93.
In the implementation, if it's #if 0 it's almost always debug assist code which I turn on from time to time to help fix some reported bug. Or it's code which isn't fully baked yet and end users shouldn't use it e.g. byte range locking.
* I doubt that std::cerr should be in this library's code.
Use of std::cerr is only when something truly unusual occurs, like we are about to fatal exit the process and I would assume the user would like to know why we just called std::terminate.
Please, don't do that (both print anything and call std::terminate). In my projects I'm very often required to remove any output to the console and redirect it to a (custom) log library. Third party libraries that have hard-coded console output are always a pain and have to be patched. Calling std::terminate is also something I would call unexpected and harmful. You don't know what the application is doing - one thread may be doing something with Boost.AFIO and another may be doing domething important and unrelated. Throw exceptions on errors, where possible. Document UB when not.
Or we are about to deadlock, and again the user probably wants to know why their application has just hanged itself.
Let it deadlock. Seriously. At least the user will be able to see it blocked and get a backtrace to figure out how did it come to this. Terminating the app is the most useless and harmful reaction.