Re: [boost] [gsoc-2013] Boost.Expected

Subject: Re: [boost] [gsoc-2013] Boost.Expected
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2013-04-13 07:23:06

Le 13/04/13 08:38, Pierre T. a écrit :
> Hello,
>
> My name is Pierre Talbot, I participated to GSoC 2011 and began the 
> implementation of the Boost.Check library (for checking the validity 
> of any number having a check digit — credit card number, ISBN, …). It 
> was two years ago and the design is still evolving due to many 
> variation points in the code. One of the function looks like :
>
> template  <typename  check_algo,
>           typename  range>
> boost::optional<typename  check_algo::checkdigit_type>
> compute_checkdigit(const  range  &x);
>
> Using boost::optional tell us that: "A check digit should be computed 
> if 'x' is a valid sequence". Since 'x' has many reasons to be 
> incorrect, many errors could be raised. Based on a policy template 
> class, it launches exception or returns with an empty optional . Then 
> I though a lot about a better way to do it, allowing the user to get 
> an exception or an error code. But it was quite complex for a so 
> little part of my library… I decided that the policy "throw exception 
> or nothing" should be enough.
>
> Yesterday, I watched the video of Mr. Alexandrescu on Boost.Expected 
> and I think it would be a very useful library. It could mainly be 
> useful in system programming (where many error codes can arise from a 
> single call), but in any code where many exception errors can be 
> thrown (like in Boost.Check).
>
> As you suspected, I'm interested in coding Boost.Expected during the 
> summer as a GSoC student.
>
Great.
> Firstly, it could be very useful to list the resources on the subject 
> (aside the talk), I have several articles that I will talk about later.
>
> Secondly, and hoping you'll debate, I would like to ask your opinion 
> about several ideas and facts:
>
> 1) In the Boost project description, we can read: "adding a class 
> expected-or-error_code". Obviously, the main design decision made by 
> Alexandrescu is to consider that error code are exception. Do we need 
> an abstraction of exception/error code ? Why do you think a 
> "expected-or-error_code" class is interesting ?
>
During discussion on how to implement c++ concurrent queue [1] it was 
suggested in [2] that a value-or-status class will help to make the 
interface more functional.

[1] http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3533.html
[2] 
http://www.google.com/url?sa=t&rct=j&q=&esrc=s&source=web&cd=1&ved=0CDQQFjAA&url=http%3A%2F%2Fgroups.google.com%2Fa%2Fisocpp.org%2Fgroup%2Fstd-proposals%2Fbrowse_thread%2Fthread%2F83db324ec26a0ab1%2Fb0cf6422f8d95803%3Fshow_docid%3Db0cf6422f8d95803&ei=Lz1pUYD1IJOR0QWtloDwDg&usg=AFQjCNFGiNLdGOaX9cse2vkRmTiXEUS5BA&sig2=Ho882VjpxYDrpnZow5wmxg&bvm=bv.45175338,d.d2k

instead of having

void pop(T&);
queue_op_status try_pop(T&);
T value_pop();

we could just have a

value-or-status pop();

This class is very close to Expected, but instead of storing an 
exception, it stores the status (or error_code). It is a class that is 
more adapted to lower interfaces that need to check the error code and 
handle the error and for libraries that find expensive to throw an 
exception or just can-not use exceptions. I would say that this class is 
a must to have in the library.

> 2) Consider this code (from Alexandrescu slides) :
>
> // Caller
> string s = readline();
> auto x = parseInt(s).get(); // throw on error
> auto y = parseInt(s); // won’t throw
> if (!y.valid()) {
> // handle locally
> if (y.hasException<std::invalid_argument>()) { // ------------------ < 
> The flagged line.
> // no digits
> ...
> }
> y.get(); // just "re"throw
> }
>
> The flagged line has some tastes of "return to the past" flavor. Back 
> to the C procedural language, the basic error code handling system was 
> a lot criticized because:
>
> * Readability of the code decrease ;
> * Error handling occurs in the middle of the execution flow ;
> * <Add your favourite reason here>.
>
Well as you know this is a question of taste and taste can not be discussed.
> Several links on the subjects (if you have others, I'm interested)
>
> http://www.joelonsoftware.com/articles/Wrong.html
> http://www.joelonsoftware.com/items/2003/10/13.html
> http://blogs.msdn.com/b/oldnewthing/archive/2005/01/14/352949.aspx
> http://nedbatchelder.com/text/exceptions-vs-status.html
I don't find any more the slides of Alexandrescu's presentation. I have 
them so if you can not find them neither I can send you them privately.
I 'll take a look to this links soon.
>
> Basically, only the last one is clearly for exception. The main 
> argument against the procedural approach is the readability. I would 
> say that the Expected approach just differ by allowing to rethrow 
> exception. But if you want to handle it, you must code multiple 
> if-than-else statements.
Agreed. expected can be see as a variant at two levels. The fist one 
contains a value or an exception_ptr. So accept_visitor could have a 
sens. In addition exception_ptr is some kind of any exception where the 
exceptions can for a hierarchy. So any kind of hierarchical 
accept_visitor could be applied as well.

Just note that the expected-or-error code can use a switch. No need for 
if-then-else.
>
> So I considered a complementary approach working with Expected to 
> handle multiple error cases:
>
> string visa_number = readline();
> expected<char> expected_checkdigit = 
> compute_checkdigit<visa>(visa_number);
> if(expected_checkdigit.valid(visa_error_resolver))
> {
> visa_number += expected_checkdigit.get();
> std::cout << visa_number << std::endl;
> }
>
> With this code, there is only a if statement, and no more multiple 
> error cases handles. But what is this error_resolver ?
>
> It may be declared as :
>
> // Somewhere in visa.hpp. A type list.
> typedef error_list<size_error_exception, unknown_character_exception, 
> checkdigit_encoding_exception, …> visa_errors;
>
> // Somewhere in the user code.
> error_resolver<visa_errors, expected_type> visa_error_resolver; // in 
> this case, expected_type is a char.
>
> // Initialize error handler on specific exception/errors.
> visa_error_resolver.on<size_error_exception>(size_error_handler)
> .on<unknown_character_exception>(unknown_character_exception)
> ...
>
This approach and other forms of the hierarchical visitor pattern can be 
considered of course. I would not however use it with the valid function 
but with a specific visit function.

if(expected_checkdigit.valid())
{
visa_number += expected_checkdigit.get();
std::cout << visa_number << std::endl;
} else {
   expected_checkdigit.visit(visa_error_resolver);
}

In addition this visitor could be applied to any exception_ptr, so we 
can even provide it independently of the expected library as far as 
expected provide access to the exception_ptr.

   visit(expected_checkdigit.get_exception_ptr(), visa_error_resolver);

> Now we are agree that visa_error_resolver can be reused everywhere we 
> want to resolve an error on a visa number.
>
> What are the handlers ? There are [Function|Functor|Lambda] (pick up 
> your favourite) with this form :
>
> expected<ExpectedType> size_error_handler(const size_error_exception&)
> expected<ExpectedType> unknown_character_exception(const 
> unknown_character_exception&)
>
> Now you can understand for what the type list "error_list" stands for, 
> we can store these handlers into the error_resolver and call them 
> without any virtual cost.
I wold prefer to maintain expected<> as simple as possible and don't add 
anything that is not mandatory.  But of course this is your talk.
>
> Why the return type of error handler is expected<ExpectedType> ?
>
> Consider this size_error_handler code :
>
> expected<ReturnType> size_error_handler(const size_error_exception& e)
> {
> std::cout << "The number you gave has a bad size." << std::endl;
> std::cout << "Enter it again : " << std::endl;
> return read_visa_checkdigit();
> }
>
> read_visa_checkdigit can call recursively valid() until it's valid. 
> Though there are some ways to make this treatment iterative.
>
> A basic treatment could be to print an warning message and just 
> returns (in this case, valid returns false):
>
> expected<ReturnType> size_error_handler(const size_error_exception& e)
> {
> std::cout << "Warning: the VISA field is incorrect." << std::endl;
> return expected<ReturnType>::fromException(e);
> }
>
> Results:
> * The error code handling is delegated to specific functions ;
> * The readability is still excellent ;
> * You can easily re-use your error handler function ;
> * If you don't like it, you can still throw exception on failure with 
> "get()".
>
> I can code a "proof of concept" if you think this is a good idea.
I have started a basic implementation of expected<>. I would share it 
with you soon. Maybe you can start to prototype the visitor pattern 
applied to an exception_ptr.
> Do not hesitate to comment it, point out programming pitfalls, request 
> further clarification, or anything you judge useful.
>
> Thank you for reading it !
>
I see you have a lot of excellent ideas. The fist thing to do is to make 
a concrete proposal. Put all of what you have in mind. Make a realistic 
proposal, a plan and ensure that you will be able to make ready for 
review the library an the end of the GSoC period. I would not mentor a 
proposal that has to propose for review to the Boost community at the 
end of the summer.

Maybe other mentors could appreciate your proposal even if it don't 
satisfy my constraints, so make the proposal you are confident with, at 
the end it is youself that would work on it not the mentor.

Good luck,
Vicente