Subject: Re: [boost] [AFIO] Review (or lack of it)
From: Niall Douglas (s_sourceforge_at_[hidden])
Date: 2015-08-28 19:07:58
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
But fair enough it could be interpreted differently, so logged to
> >> 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
> 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
> >> * 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.
-- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk