On 28 Aug 2015 at 8:29, Roland Bock wrote:
I am unaware of any case where I imported a namespace except into a local context. Copied from the introduction page:
using namespace BOOST_AFIO_V2_NAMESPACE;
That isn't a global using namespace. It could be a local using namespace, and in its original source file it is indeed a local using namespace. But fair enough it could be interpreted differently, so logged to https://github.com/BoostGSoC13/boost.afio/issues/102.
Why is there a writable flag (why isn't constness enough to prevent writing)? I have no idea what you're going on about here, but constness at the language level has nothing to do with whether you open an on-disk store for reading or writing. First of all, please reread your claim: "World's simplest named blob store in STL iostreams"
What you present is definitely not what you promise.
By simplest it was meant the public interface API. Not the implementation. I thought that obvious, but logged anyway to https://github.com/BoostGSoC13/boost.afio/issues/103.
Also, git has branches. Why don't you work on the half-baked stuff in a feature branch?
#if 0 is code smell.
These are entirely personal opinion based on suppositions and assumptions with zero technical relevance to the quality of a library, as was a lot of the stuff I snipped. I don't work in pristine code environments, I like to keep around bits of code in #if 0 blocks to remind me of things, or for the occasional use, or for any other reason. I personally would find such segments educational when reviewing a library rather than the unfounded claim of "code smell" which may be true in other code bases in your experience, but I can assure you is not the case in my code bases. Instead of assuming code smells or other non-factually based suppositions, consider examining the coverage of the unit test suite and the rigorous testing regime run each and every commit and taking some hours to complete.
* 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. Or we are about to deadlock, and again the user probably wants to know why their application has just hanged itself. Quite a few applications turn off or ignore std::cerr. At least given users the option to provide their own reporting function for emergencies.
It is trivial to redirect std::cerr elsewhere. I am happy for std::cerr to become a macro, and this is logged at https://github.com/BoostGSoC13/boost.afio/issues/104.
* I looked briefly into detail/. I would not want to look for a bug there. I have not done any detailed analysis to base this on, but: I would assume that with some refactoring (moving the platform specific parts into platform specific headers), the readability could be increased dramatically. Again: This is just a guess. The platform specific parts are already in platform specific files. afio.ipp is for POSIX which includes MSVCRT. afio_iocp.ipp is for Windows IOCP. Hmm. The code I looked at was still riddled with #if THIS_SYSTEM do that #else do something else.
Sure. There are substantial deviations between POSIX implementations, but not enough to warrant separate files. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/