
From: "Jonathan Turkanis" <technews@kangaroologic.com>
Rob Stewart wrote:
From: "Jonathan Turkanis" <technews@kangaroologic.com>
expect I will have to introduce some new Device concepts. However, I would like to modify the *current* filter concepts so that they will work unchanged when non-blocking and asynchronous devices are introduced.
There's a difference between making the concepts work unchanged and making the components of the library work unchanged.
True. I can always fix the library-provided component so that they do the right thing, even if I have to rely on magic, i.e., on an incestuous relationship with library internals.
If the library is well-received, however, I expect eventually there will be a large body of user-defined filters, and I wouldn't want them to have to be rewritten.
Given the filters written with sync I/O in mind are quite different from those written for async I/O, is that really an issue?
- Instead of returning void, write() can return an integer indicating the number of characters written.
But it also needs to indicate errors.
Errors are indicated with exceptions.
(This is another topic that didn't get much attention during review. There are some, notably James Kanze, who insist that well-designed stream buffers should not throw exceptions. My defense of exceptions is here: http://tinyurl.com/59xv6. At the time of the review, I was prepared to switch to error codes, but it's a bit late now.)
I apparently missed that during the review. On what basis does James Kanze make his assertion? So long as EOF and EAGAIN do not result in exceptions, given how commonly they occur, I have no problems with exceptions. However, it sounds like you might be throwing an exception to indicate EOF. If so, I don't like it. It is not entirely uncommon to read to EOF, do something to a file, and continue reading. For example, we have certain files that we tail throughout the day in our production apps. EOF is a common occurence because we quickly process the new data appended to the files and reach EOF again. That is, the sink is faster than the source. This behavior would be complicated by EOF throwing an exception. An error indication of EOF, plus the ability to clear that condition and try again, fits this usage better. (Granted, that use of a file is not widespread, but since you offer no choice, such usage is not possible.) You mentioned in the cited documentation section that you didn't want to complicate things such that users would wonder which of the various ways a given function indicates errors. I sympathize, but isn't that what you're introducing with your basic_character idea? How about putting a state variable in filters, sources, and sinks to indicate EAGAIN, EOF, and GOOD. Then, functions that only read or write a single character can return a bool, and functions that read or write multiple characters can return a count. If the bool is false or the count is zero, the code can query the state to determine whether EAGAIN or EOF occurred. That keeps the error indication in the return value simple and even leaves room for adding additional error states should that prove necessary. Hard errors can still be signaled via exception.
V. Problems ----------------------------
1. Harder to learn. Currently the function get and the concept InputFilter are very easy to explain. I'm afraid having to understand the basic_character template before learning these functions will discourage people from using the library.
The class template is hardly complicated. I can't imagine it would be a show stopper, though it does add some complexity.
I'm thinking of decribing it as a replacement for traits_type::int_type. Compared to using int_type and its family of helper functions (eq_int_type, eof., not_eof, ...) basic_character is a snap. ;-)
That's a good way to make it palatable to those already familiar with writing streambufs, but it hardly applies to the majority of your library audience, does it.
2. Harder to use. Having to check for eof and fail make writing simple filters, like the above, slightly harder. I'm worried that the effect on more complex filters may be even worse. This applies not just to get, but to the other functions as well, since their returns values will require more careful examination.
That's a real issue.
It's become even more clear in this thread, given my botched uncommenting_filter implementation.
Keep running notes on the things one can forget or do wrong so your documentation can warn about them.
That way, those writing synchronous code can write it using a character type of char/wchar_t and everything to/from that code will be in the simplified, synchronous style you currently offer. However, if the client takes advantage of the advanced, asynchronous capabilities of the library by using basic_character, then the style changes based upon needing to deal with the EAGAIN condition.
If I adopt this solution, I'll be writing non-blocking versions of all the library filters and so may be able to judge whether there is a big stylistic difference, and whether it would be a good idea or even possible to modify the current concepts.
That sounds good.
(The member functions good(), eof(), and fail() on basic_character could be made non-member functions and could be implemented for char and wchar_t. That would help code deal with synchronous code in the asynchronous style.)
I guess I could implement
bool eof(int n) { return n == EOF; } bool good(int n) { return n != EOF; }
bool weof(std::char_traits<wchar_t>::int_type n) { return n == WEOF; } bool wgood(std::char_traits<wchar_t>::int_type n) { return n != WEOF; }
I tend to think that n == EOF and n == WEOF are more readable, however.
Yes, they are more readable, but if you don't document the implementation of those functions, library users will expect to use them in all situations. Thus, switching to async from sync I/O will be less of a stylistic change. (That also leaves room for you to change the implementation details if need be.) Of course, if you adopt my state idea, then the return value would simply indicate that things went well or something less than ideal happened. In the latter case, one would query the state of the filter/device to learn what happened. That would obviate your basic_character class and these functions. One problem with code that returns status information is that folks can forget to check the status. You could return a special type in lieu of bool, and basic_character in lieu of char_type, which require inspecting whether they indicate an error condition. If that query is not done, the destructor can complain. OTOH, you could just say, "don't do that!" -- Rob Stewart stewart@sig.com Software Engineer http://www.sig.com Susquehanna International Group, LLP using std::disclaimer;