On 08/29/2015 02:05 AM, Niall Douglas wrote:
On 28 Aug 2015 at 10:16, Thomas Heller wrote:
I took the effort to also reimplement your hello world example: https://gist.github.com/sithhell/f521ea0d818d168d6b35
Firstly thank you very much for your gist. This makes it much easier to understand what you are talking about.
My very first thought was that you are using reference capturing lambdas in a concurrent use case which may be fine in internal implementation, but is a serious no-no to ever place upon a library's end users. People regularly get waits and wakeups wrong, and I have seen code bases wrecked by memory corruption and race conditions induced by reference capturing lambdas used in concurrent situations.
All true. This contrived example is not really concurrent though, but whatever. Please, stop treating your users as they wouldn't know what they do.
For me this is an absolute red line which cannot be crossed. I will never, *ever* agree to a library API design which makes end users even feel tempted to use reference capturing semantics in a concurrency context. Period. They are banned as they are nuclear bombs in the hands of the typical C++ programmer.
Sure, whatever, this is just derailing ... You do realize though that you demote your users library to idiots not knowing what they do? Despite promoting your library as something very niche only used by experts? This is by giving up your own promoted "Don't pay for what you don't use" principle. But let's move on...
So let's replace all your reference captures with shared_ptr or value captures and thereby making them safe. I have forked your gist with these changes to:
https://gist.github.com/ned14/3581d10eacb6a6dd34bf
As I mentioned earlier, any of the AFIO async_* free functions just expand into a future.then(detail::async_*) continuation which is exactly what you've written. So let me collapse those for you:
Where does error handling happen here? How would the user react to errors? What does depends do (not really clear from the documentation)? How do I handle more than one "precondition"? How do I handle "preconditions" which have nothing to do with file I/O? Which ultimately leads me to the question: Why not just let the user standard wait composure mechanisms and let the functions have arguments that they really operate on? All questions would have a simple answer then. NB: I am still not sure what afio::future<>::then really implies, does it invalidate the future now? When will the continuation be executed, when the handler future is ready or when the "value" future is ready? What does that mean for my precondition?
This is almost identical to my second preference API for AFIO, and the one Gavin Lambert suggested in another thread.
As I mentioned in that thread with Gavin, I have no problem at all with this API design apart from the fact you need to type a lot of depends() and I suspect it will be a source of unenforced programmer error. I feel the AFIO API as submitted has a nice default behaviour which makes it easy to follow the logic being implemented. Any depends() which is a formal announcement that we are departing from the default stand out like a sore thumb which draws the eye to giving it special attention as there is a fork in the default logic.
That's exactly the problem: You have a default logic for something that isn't really necessary, IMHO. The async operation isn't an operation on the future, but on the handle. And to repeat myself: That's my problem with that design decision.
Under your API instead, sure you get unique futures and shared futures and that's all very nice and everything. But what does the end user gain from it?
1. Clear expression of intent 2. Usage of standard utilities for wait composures 3. Transparent error handling 4. "Don't pay for something you don't use"
They see a lot more verbiage on the screen. They find it harder to follow the flow of the logic of the operations and therefore spot bugs or maintain the logic.
Verbosity isn't always a bad thing. Instead of trying to guess what is the best default for your users stop treating them as they would not know what they do.
I am seeing lots of losses and few gains here apart from meeting some supposed design principle about single responsibility which in my opinion is a useful rule of thumb for inexperienced programmers, but past that isn't particularly important.
You do realize that this is highly contradicting to anything else you just said in your mail? You claim to have your library designed for the inexperienced user not knowing what they do, yet your design choice violates principles that are a nice "rule of thumb for inexperienced programmers"? Beside that, single responsibility should *always* be pursued. It makes your code testable, composable and easier to reason about.
I however am probably being a little unfair here. I reduced your original to something AFIO like and something I previously gave deep consideration to adopting before rejecting it, and you were probably actually advocating that people manually write out all the .then() logic instead of using free functions which expand into exactly the same thing.
Right, dependency handling is something inherhent to the users code. Anything you'll come up with inside of your library will be the wrong thing in some cases and then you'll have a problem.
If you really strongly feel that writing out the logic using .then() is very important, I can add that no problem. Right now the continuation thunk types live inside a detail namespace, but those can be moved to public with very little work.
Niall