RE: [boost] Re: Formal Review for Interval Library beginning

From: Rozental, Gennadiy (Gennadiy_at_[hidden])
Date: 2002-09-03 12:46:18

> I agree with you concerning the last point; named template 
> parameters can
> be interesting. But I'm not sure it's what the users will principally
> used. I think they will use things like:
> 
>   typedef change_compare_policy
>             <my_old_interval_type, my_new_compare_policy>
>           my_new_interval_type;

No. I thought more like

interval<T, rounding_is<some_rounding> >

> The reason it is in the  public section is
> only an artifact of the way the library interface is designed and the
> compilers are conforming.

It should be in private section at least for conforming compilers. Use some
kind of ifdef switch and add private and friend declarations optionally.

> > 4. Why do you need set_empty and set_whole. Why could not you use "=
> > interval::empty()" and "= interval whole()"
> 
> As for set before, if you refer to the documentation, you 
> will see that
> set_empty and set_whole are not part of the published interface of the
> class.

I does not change my point. Why would you need excessive function in your
(private) interface?

> 
> > 5. interval free function interface/implementation IMO 
> should be split up in
> > several independent headers. For example In many cases I 
> may not need
> > trigonometric, transcendental and so forth functions over 
> intervals. It may
> > also allow to minimize dependency on STL headers cmath and algorithm
> 
> Yes, it is a possibility.

Look for example on MPL. Almost Every function in separate header. It could
be an overkill for you, but one solid header is even worse.

> I don't think it is possible to write the same class if the 
> methods are
> static. There is a lot of situation where it is necessary to 
> access local
> data.

Ok. I just thought that rounding policies are always stateless.

> > 2. Boost recommend to use T const instead of const T
> 
> Ok. (if you remember where it is written, could you give me a
> pointer? thank you)

See coding guideline on boost site.

> > 4. using std::log and myriads of other symbols is all over 
> rounding headers.
> > What about the compilers that does not put the into std namespace?
> 
> It's a big problem. It's the reason why 
> interval/detail/bug.hpp tests if
> BOOST_NO_STDC_NAMESPACE is defined.

Does not it too limiting. How many compilers you are cut off by this?
Interval library implementation does not use too much advanced C++ tricks so
it could work on wide range of compilers.

> > 5. Why interval implementation is located in utility.hpp 
> header. It may be
> > misleading.
> 
> If it is really disturbing, it can easily be moved elsewhere.

It would be more clear IMO.

> 
> > 6. There are some commented lines. Since we are using cvs 
> may be it worth to
> > clean the code?
> 
> Yes it is worth to clean the code of the few remaining commented
> lines. But what do you mean by "Since we are using cvs" ?

People usually put if 0 or comment code cause they do not want to lose the
information. Since we are using cvs no need to bother - cvs will take care
about this.

> 
> > Docs:
> > 1. Heading: May be it's worth to name the page Interval library (or
> > something like this) and put reference to the definition separately
> > somewhere.
> > 2. On top of the page right under he header there is TOC. 
> IMO it is aligned
> > a bit strangely and it would look better if you delete <center> tag
> 
> Nothing much to say.

Does it mean you agree?

> 
> > 3. Introduction, statement 2: "It consists of a single 
> header". It is not
> > true.
> 
> Yes. The reason of this error is that only one header is needed to be
> included in order to access the major part of the library.

It still would be better to rephrase it.

> > 4. Introduction, statement 3: "Traits is a policy".
> Yes, you are right. And it's reason why we speak of 
> "policies" and not of
> "traits" in the documentation. However, as you may know, this 
> library was
> originally the work of Jens Maurer, and by respect to that 
> work, we tried
> to limit the number of naming changes to a minimum.

IMO it is *very* important to use proper terminology. When I see template
parameter named traits I am confused.

> Some concept checking would be nice. But I'm not sure it would be that
> useful (am I underestimating the strength of concept checking 
> in the case
> of interval arithmetic?)

I think it could be useful.

Gennadiy.