Re: [boost] [review] string convert

Subject: Re: [boost] [review] string convert
From: Vladimir Batov (vbatov_at_[hidden])
Date: 2011-05-05 10:50:27

> "Stewart, Robert" <Robert.Stewart_at_[hidden]> wrote 
> 
> In another post, I offered the following interface:
> 
> > 1. T convert_cast<T,S>(S)
> > 2. T convert_cast<T,S>(S, T)
> > 3. T convert_cast<T,S>(S, T, nothrow_t)
> > 4. optional<T> try_convert_cast<T,S>(S)
> > 5. optional<T> try_convert_cast<T,S>(S, T)
> >
> > In 1, the result valid unless there's an exception.  It doesn't
> > work for non-DefaultConstructible UDTs.
> > In 2, the result is always valid since it never throws.  It
> > works for non-DefaultConstructible UDTs, but not for types
> > without a suitable fallback value.

#2 seems to have to be

2. T convert_cast<T,S,U>(S, U, identity<T>::type)

Where U is the type of the fallback value. It obviously can be different from
the target type. For example,

// fallback is char const*
std:string str = convert_cast<std::string>(123, "failed"); 
// fallback is int
double d = convert_cast<double>("123", 0);

> > In 3, the result is valid unless there's an exception.  2 and 3
> > could be merged, but keeping them distinct is probably more
> > efficient and should be easier to document.

As written at the top up #2 and #3 seem to do the same thing. Namely, both do
not throw (with nothrow_t in #3) and return fallback on failure. Above though
you say "In 3, the result is valid unless there's an exception". Could that be
that you meant

3. T convert_cast<T,S>(S, T, DOthrow_t)

If so, it'll be something like the following, right?

3. T convert_cast<T,S,U>(S, U, DOthrow_t, identity<T>::type)

> > In 4 and 5, the optional is set iff the conversion succeeds.
> > 4 is useful for built-in types and DefaultConstructible UDTs.
> > 5 is useful for all types.

#4 seems to be a non-throwing version of #1, right? #1 and #4 seem like a nice
complementary pair for DefaultConstructible types and tasks currently covered
"natively" by lexical_cast but with the choice of non-throwing (already a plus). 

Then, as a user I instinctively try pairing up #2 and #5 and fail. #1 and #4
exhibit the same behavior but throws and not. #2 does not throw already. Then, I
think #5 must complement #2 somehow differently. #2 does not indicate failure.
Then, #5 probably does. Yes, it does... but #5 *never* returns the fallback
value (as #2 does) that I provided (which might be seen as surprising). 

The fact is that #5 is not really a comlepement for #2. More so, the supplied
value is not a fallback either but to work-around #4 limitations (for
non-DefaultConstructible types). 

IMHO that situation (when the provided value is sometimes a fallback sometimes
not) could be potentially quite surprising for the user.  

Maybe to avoid the temptation of pairing up we could simplify (at least from the
user perspectives) down to:

1. T convert_cast<T,S>(S)
2. T convert_cast<T,S>(S, T, throw_t =nothrow)
5. optional<T> try_convert_cast<T,S>(S, T =T())

Still, the difference in the meaning of the second parameter in #2 and #5 makes
me feel uncomfortable.

> Your use case is the following, right?
> 
> optional<int> o(try_convert_cast<int>(str));
> int i;
> if (!o)
> {
>    std::cout << "using fallback\n";
>    i = fallback;
> }
> else
> {
>    i = o.get();
> }
> 
> That's clearly inconvenient and 5 doesn't apply because the optional won't
> be set if the conversion fails.  To address such inconvenience, you offered
> convert<T>::result and pair<optional<T>,bool> has been suggested.  Isn't
> optional redundant in the latter?  

Yes, indeed, std::optional inside std::pair is/was redundant.

... Purely for brevity I snipped your example which was explaining redundancy of
optional.

> I noted one missing use case:
> 
> > What's missing, then, is getting an exception when a conversion
> > to a non-DefaultConstructible UDT or a type with no fallback
> > value fails.  That is, a variant of 2 that throws on conversion
> > failure.
> 
> Possibly, that would just mean:
> 
> 2a. T convert_cast<T, S>(S, T, throw_t)

I am under impression that the following covers all use-cases (known to me so
far)

1. T convert_cast<T,S>(S)
2. T convert_cast<T,S>(S, T, throw_t =nothrow)
6. std::pair<T, bool> try_convert_cast<T,S>(S, T =T())

#1 is straightforward lexical_cast replacement. #2 takes fallback and returns
it. Optionally throws on request. #6 replaces the original #5. Never throws.

In fact, it is not unreasonable (as you mentioned) to say that we provide a
building block library. That way we'll only need to supply #6 as #1 and #2 are
easily covered:

#1
std::pair<T, bool> res = try_convert_cast<T>(s); 
If (!res.second) throw an ex. of your choice

#2
T v = try_convert_cast<T>(s).first;

[...] Snipped some more. It's 1AM and I want to get to the bottom of it. :-)

> If we use Vicente's default_value customization point, however, things
> could be interpreted a little differently:
> 
> a) T convert_cast<T,S>(S)
> b) T convert_cast<T,S>(S, T)
> c) optional<T> try_convert_cast<T,S>(S)
> d) optional<T> try_convert_cast<T,S>(S, T)
> 
> a) Uses default_value<T>, if needed, and throws on conversion failure.
> b) Conversion failure implies returning the second argument's value.
> c) Uses default_value<T>, if needed, and the return value is not set on
> conversion failure.  This still needs a better name.
> d) Conversion failure implies returning the second argument's value.  This
> still needs a better name.
> 
> Given the relative rarity of types that need a special "default" value, and
> the fact that a compilation error on a line in the primary specialization of
> default_value can lead the library user to a comment that explains the need
> to specialize it for T, the regularity of a-d is compelling.

It's probably getting late and my brain is not working but I cannot help
thinking that the three choices I mentioned above (or maybe even just one) would
cover all the cases. I'll revisit it again.

> Three things are missing from a-d.  One is formatting control.  However, that
> can be done through extra arguments as suggested in various other posts.  It
> may be that there would even be overloads of a-d that take the additional
> arguments in order to streamline the simpler cases.

Supplying formatting, locales, etc. might be done with boost::parameter. It
takes some getting used to though.

> Another thing missing from a-d is the function object you had in 
> convert<T>::converter<S>, IIRC.  Is there any reason that cannot just be 
> captured by converter<T,S>?  For simplicity of customizing the library, I'd 
> even expect that a-d would use a converter<T,S>.

The main reason the converter existed was the need to gather configuration
information before applying actual conversion as the configuration process
(providing locale, manipulators, etc.) was gradual. With this design I feel all
the information -- value-in, fallback-value, configuration -- needs to be
supplied in one long "sausage". It could have a signature similar to

T convert_cast<T,S>(S, (boost::parameter-list), identity<T>::type)
i.e.
int v = convert_cast<int>("FF", (format_ = std::hex, locale_ = locale, throw_ =
yes))

I think it's quite acceptable.

As for feeding all that to algorithms, then OTOH I am not sure how to achieve
that as we'll need a proxy holding all configuration parameters together.

> Finally, the function to address your use case is missing.  It needs a
> suitable name, too.

Well, it's not exactly *my* case. I happen to bring it forward as some people
who do not deal with, say, XML parsing tend to find it surprising when for XML
it's quite standard processing (where an element/attribute are optional but one
needs something (fallback) to proceed anyway).

> Summarizing, then:
> 
>  - default_value<T> customization point
>  - converter<T,S> customization point; main logic
>  - T convert_cast<T,S>(S, formatting = none); can throw
>  - T convert_cast<T,S>(S, T, formatting = none)
>  - optional<T> name_me_1<T,S>(S, formatting = none)
>  - optional<T> name_me_1<T,S>(S, T, formatting = none)
>  - pair<T,int> name_me_2<T,S>(S, T, formatting = none)
> 
> Note that the T argument for convert_cast<T,S>(S, T, formatting) is 
> non-deducible in order to require specifying T in each call.  Since the 
> name_me_1 and name_me_2 names are not expected to end with "_cast," the T 
> arguments may be deducible.

1) Again, as I mentioned before (which can be wrong) I feel that the list of
functions can be reduced maybe even down to just one (returning std::pair) if we
want to keep the lib. minimal and consider additional functionality (like
throwing) to be orthogonal and outside of the scope of the lib.

2) I suspect T can not be deducible in any of the cases above as I might provide
"char const*" type as a fallback and expect std::string in return. 

Going to bed now,
V.