Re: [boost] [XInt] Review

Subject: Re: [boost] [XInt] Review
From: Chad Nelson (chad.thecomfychair_at_[hidden])
Date: 2011-03-09 20:59:00

On Wed, 9 Mar 2011 19:11:31 +0100
Joachim Faulhaber <afojgo_at_[hidden]> wrote:

> This is my review on the XInt library. [...]

Thank you for the review, and the comments. I'm going to try to be
brief in my replies. No offense intended, time is just constrained
tonight.

> 1.3 Naming: Types
> =================
>
> What I like about boost, compared to many other libraries, is the
> culture of short and concise naming.

While I can appreciate that from a typing point of view, the trend
since the seventies is clearly toward more descriptive names. A name
that's both short and descriptive is obviously better than one that is
neither, but when conciseness interferes with descriptiveness, the
general consensus seems to be that descriptiveness should win.

However, typedefs offer a way to have both: a real library name that's
descriptive and possibly long, and a concise version for your own use.

> This includes the absence of cryptic pre and suffixes. But there are
> exceptions. One of those is the _t suffix used for a variety of
> types, namely int<n>_t types from Boost.Integer. [...]

I find that it's useful to denote types, especially when the name that
you're using is the most logical one for variables of the type. But if
you're referring specifically to integer_t, the problem is moot: it
will be renamed to basic_integer before the next release.

>2.2 Boost.Parameter considered unfavorable
>==========================================
>
> Although flexible, the options feature based on Boost.Parameter comes
> with an unnecessary and undesired level of abstraction and
> indirection. It obfuscates the different kinds of parameters and makes
> the implementation much more difficult to read. Moreover the meta code
> involved with Boost.Parameter imposes a compile time penalty. [...]

As the number of options grows, it becomes harder to use the type
without Boost.Parameter or something similar. I feel that XInt has
passed the threshold.

> Also
>
> integers::fixed<512> myVar; //is very intuitive, while
>
> xint::integer_t<xint::options::fixedlength<512> > myVar;
>   // can't probably be done without consulting the docs
>   // and it is more verbose

Is that really a problem? You'll likely look at the docs once for each
program you write, create a typedef for the integer type you want, and
then ignore them entirely. Although the latter example is more verbose
in this case, if you used more options it would quickly become
preferable. Especially from the point of view of a reader, and code is
read a lot more often than it's written.

> 2.3 Policy based design
> =======================
>
> I think policy based design is a very good and powerful technique that
> is specifically useful to separate implementation variants of class
> templates in clear and flexible ways. Unfortunately I can not see that
> you are actually using policy based design here. Because your design
> lacks typical policy classes that encapsulate implementation variants
> in a way that reduces codereplication and enhances flexibility. [...]

As I'm not sure what kind of implementation you're thinking of, I can't
respond to this. So far as I can tell, my design is doing what you
describe for policy-based design.

> There is a great deal of code replication in the current
> implementation of xint::integer_t. [...] This skeleton (and some
> variations of it) occurs over and over again only to propagate the
> call of the function to the implementing class at positions #1 and #2.

integer_t handles some options, and passes the data to the implementing
class for others. It was the best solution I could find to handle all
options.

> 3.2.1 Operator template overloading conflicts
> =============================================
>
> In integer.hpp 2399 and 2426, there are template macros with an
> unrestricted template parameter 'typename N' [...]

Already dealt with for the next release.
-- 
Chad Nelson
Oak Circle Software, Inc.
*
*
*