Re: [boost] [xint] Boost.XInt formal review

Subject: Re: [boost] [xint] Boost.XInt formal review
From: Chad Nelson (chad.thecomfychair_at_[hidden])
Date: 2011-03-07 11:48:48

On Mon, 7 Mar 2011 09:43:46 -0500
"Stewart, Robert" <Robert.Stewart_at_[hidden]> wrote:

>>>> Since I had to_string and to_binary, a set of to_integer functions
>>>> would have been ideal. But to *what* integer? [...] Making those
>>>> constructors eliminated the need to specify a type parameter to a
>>>> function [...] Why force the person using the library to type both
>>>> every time?
>>>
>>> That is certainly an issue.  auto obviates the problem, of
>>> course.
>>
>> Would it? You'd still have to specify what integer type you
>> want the to_integer function to return, wouldn't you?
>
> I was thinking of the type on the left hand side, which you weren't
> showing.  Your point is valid.  However, what you've named
> "to_integer" could be named "integer_cast" and it would look quite
> reasonable:
>
>   value = integer_cast<integer>(...)

That would still require you to specify both integer_cast and the type,
which is what I was trying to get away from.

>>> You could also pass the instance as a non-const reference
>>> argument to allow for type deduction.
>>
>> Possible, though it would require at least two lines instead of one
>> (creating the integer object, then passing it to the to_integer
>> function). Painful, from an aesthetically point of view.
>
> In your example above, value already exists [...] Obviously, in the
> case of creating a new instance, it doesn't fare so well: [...]
> Ultimately, I think the constructor is appropriate, but you might
> consider the function-style cast, too.

That might be a useful addition in a later version. I'll keep it in
mind.

>>> I prefer to ensure that clients cannot misuse a class even if that
>>> means using friendship (which can be constrained with, for example,
>>> the Advocate idiom).
>>
>> I definitely agree with making it hard to *accidentally* misuse a
>> class. But trying to make it impossible is probably misguided. Fences
>> and big warning signs around high-voltage power stations are good;
>> hermetically sealed domes are likely overkill.
>
> Calling a public member function is not accidentally misusing a
> class.  Casting away constness, or some other Machiavellian tactic, is
> abuse and there's no need to defend against that.

A public member function that starts with an inconvenient underscore,
and has code deliberately hiding it from the documentation generator,
should be a strong indication that it wasn't meant for general use,
even to developers not familiar with that Python function-naming idiom.

>>> but I cannot imagine a template parameter list hidden behind such a
>>> macro would preclude creating a typedef.
>>
>> I haven't tried it recently, but *can* you typedef a template? Not a
>> concrete class made from a template, but the template itself, with
>> parameters? If so, I'd dearly love to know the syntax. That's what I
>> was trying to find a way to do, and why I finally had to settle on
>> BOOST_XINT_APARAMS.
>
> I'm confused.  integer_t<BOOST_XINT_APARAMS> cannot be a class
> template as it is used as a parameter type and a return type.  It is a
> specialization of a class template, so it is a class.  Therefore, you
> can create a typedef for it.

xint::integer is a class, but according to the compiler,
xint::integer_t<BOOST_XINT_APARAMS> is still a class template, and it
won't let me make a typedef of it. When I try adding...

    typedef integer_t<BOOST_XINT_APARAMS> T;

...just below the class, GCC gives me twenty-four errors. The first few
complain about invalid template arguments and unknown variables A0, A1,
A2, etcetera (the contents of BOOST_XINT_APARAMS), the rest are mostly
incomprehensible to a quick glance. If I try to satisfy those first few
complaints like this...

    template<BOOST_XINT_CLASS_APARAMS>
    typedef integer_t<BOOST_XINT_APARAMS> T;

...I get "error: template declaration of ‘typedef’", followed by
another eleven incomprehensible errors.

Am I missing a syntactic trick that would let this work?

>>> I disagree with your assignment of relative evil to those things.
>>> [...]
>>
>> Then it seems that there's some disagreement in the Boost community,
>
> Now there's a shock!

:-)

>> because I added the underscore because I was explicitly told that it
>> *shouldn't* be part of the documented interface.
>
> If it is a necessary feature, then it must be exposed and documented,
> even if needed only rarely.  Warn users away all you like, of course.

The functionality is exposed, and documented, via the extra Boolean
parameter to the copy constructor.

>>> The many questions about behavior raised by the reviews and
>>> discussion should clearly show that the documentation is not yet
>>> complete.
>>
>> There's always room for improvement, even on "complete"
>> documentation. :-)
>
> We apparently have a different notion of what "complete" means.

Apparently so. I'm willing to agree to disagree on it.
-- 
Chad Nelson
Oak Circle Software, Inc.
*
*
*