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

Subject: Re: [boost] [gsoc-2013] Boost.Expected
From: Pierre T. (ptalbot_at_[hidden])
Date: 2013-04-13 10:24:30

On 04/13/2013 01:23 PM, Vicente J. Botet Escriba wrote:
> 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.
In the video comments, there is this link : 
https://skydrive.live.com/?cid=f1b8ff18a2aec5c5&id=F1B8FF18A2AEC5C5!1158&authkey=!APo6bfP5sJ8EmH4
>>
>> 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.
switch and if-then-else are quite similar, aren't they?
>>
>> 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);
This is two more lines for a very common case. We should rename 
valid(error_resolver&) for validate ? Or feel free to propose any 
suggestions.
>
>> 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.
I'm not sure that my explanation is clear. In fact these two functions 
are not related to expected<>. They are the visitor.
>>
>> 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.
I have the Alexandrescu class, copied from the slides. But I would be 
glad to see yours too.
>> 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

Before going further with an implementation try, I would like to have 
your opinion on this simple idea:

I'm not sure that expected should encapsulate an exception. Moreover the 
std::exception_pointer introduces virtuality that could be quite 
intrusive for the low-level interface.
I propose that expected use a type list of error such as :

typedef ErrorList<BadLengthError, OverflowError, …> MyErrorList;
typedef expected<T, MyErrorList> ExpectedReturnType;

ExpectedReturnType value = …;

* The visitor could have the advantage to be static.
* The user can put error class, exception error class. We could 
specialize it for error code too I guess.

I have further facilities ideas that I will show you later if you think 
that could be a good compromise.

Thanks for the comments,
Pierre Talbot