Thorsten Ottosen wrote:
"Reece Dunn" <msclrhd@hotmail.com> wrote | Thorsten Ottosen wrote: |The default behaviour is to set the | input stream fail bit, so you can check that.
Ok, I guess that goes in hand with the bahavior of the standard library (although I prefer throw by default).
could syntax like
std::cin >> io::throw_on_error() >> io::formatob( ... ) std::cin >> io::default_init_rest() >> io::formatob( ... )
be supported too.?
Possibly. I'd need to look into this.
| >3. cryptic name: formatob.hpp && formatobex; should be format_object and | >whatever the latter really means | | I was going for ease of typing :). maybe | boost::io::format/boost::io::formatex. The latter allows you to control the | delimeter type being used, so you can do:
yeah, what does "ex" mean?
"extended". A better name is probably needed and should be consistent throughout (i.e. for boost::io::wrappedfmtex, etc.) The problem is how to keep close to the original so that you know what it does and also tells you that this variant controls the delimeter type, since you can't reuse the original name due to overload resolution issues.
| I was thinking here about performance. The default type is const char * | which should be fairly flexible without providing compy penalties.
seems reasonable...just mention this stuff in the docs.
It is (in a round-about way). I have fixed in my local docs to say: "This uses const char * as its delimeter storage type."
| >5. A lot of boost::io::range() can be replaced with one Taking a Single | >Pass | >Range | | You mean from your Boost.Range library? This is something that I am | considering, but don't know enough of the library to do this yet.
Ok, basically you can replace
foo( std::pair<iterator,iterator> ); foo( T (&array)[sz] ); foo( const standard_container& ) foo( builtin-strings );
with one function taking a Range
foo( const Range& );
Great! I will replace boost::io::range and boost::io::range_t with this post-review.
| >6. STL IO: It is not obvious how insertion is done? With push_back or by | >overwriting elements? | | The container is cleared and insert is used to insert the elements into the | container.
ok, a small comment about it would be good.
Done in local docs.
| >7. why is it necessary with | > boost::io::formatob( boost::io::range( a, a + 5 )); | > could we not just say | > boost::io::range( a, a + 5 ); | | Because the boost::io::range_t class (created by boost::io::range) does not | provide input/output functionality. It is a holder for a range [first, | second) so that boost::io::formatob can process it. This is if you either | specify a std::pair (boost::io::formatob treats this as a 2-ary type and not | a range holder) or a (first, last) pair of values (since boost::io::formatob | accepts a single object).
hm...it seems to me that boost.range has all you need:
class iterator_range or class sub_range + function make_iterator_range( a, a + 5 );
I could probably benefit you code to use this instead (unless I'm overlooking something).
Nope. I'll update post-review.
| >9. I would prefer basicfmt_t to be basic_format_type or perhaps | >basic_fmt_type | >if the "fmt" shorthand is used consistently etc. That is, all the cryptic | > concatenation should be removed. | | That would make things fairly long-winded. For example: | [snip]
yeah, I know, but at least say pair_fmt instead of pairfmt etc.
Yes. This is better.
| I am not sure what you mean by concatenation.
the contraction of X_fmt to Xfmt.
Okay. I'll look at revising the affected names.
| Okay, I will bear that in mind. FYI, the above function returns: | formatob_t< T, DelimeterType, typename deducer< T, DelimeterType | >::type::outputter > | ! My rationale for not supplying the return types was to not consume more | space and not confuse readers. Marking them *implementation defined* would | be a good idea.
yeah, that would be sufficient.
Great. Regards, Reece _________________________________________________________________ Check out Election 2004 for up-to-date election news, plus voter tools and more! http://special.msn.com/msn/election2004.armx