Re: [boost] [Boost-users] [Review] ITL review starts today, February 18th

Subject: Re: [boost] [Boost-users] [Review] ITL review starts today, February 18th
From: Joachim Faulhaber (afojgo_at_[hidden])
Date: 2010-03-08 14:33:35

Hi Jeff,

thank you for your review of the ITL. You have been supportive in the
course of the development at different points giving feed back about
shortcomings and helping with code patches and useful suggestions. I
am glad to hear that you are actually using the library for real world
applications already.

2010/3/8 Jeff Flinn <TriumphSprint2000_at_[hidden]>:
>> Please always state in your review, whether you think the library should
>> be
>> accepted as a Boost library.
>
> Yes, I think the library (with the interface improvements brought out during
> the review) should be accepted into boost. I've already successfully used
> the library with it's existing interface in a commercial product.
>
>> - What is your evaluation of the design?
>
> As brought up during the course of the review period my main concerns for
> the library's interface are:
>
> 1) the bloated size of the interval class with it's dynamic bound data
> member. This is being addressed by Joachim providing an interface to allow
> use of static bound interval classes.
>
> 2) the fat interface on the interval class. In my view the myriad of
> mutators would best be replaced with free functions in terms of the
> interval's extents and return a interval passing the calculated extents to
> an interval constructor which enforces the interval's invariant(s). There
> should also be a method to construct an interval given unordered extents
> that effectively does interval(min(arg1, arg2), max(arg1, arg2)).
>
> 3) better integration of std and user defined data types en lieu of itl's
> specific types. Joachim has agreed to improvements in this area.

I think we have agreed in the various points involved.

> 4) I'd also like to see std::inserter "just work" with the containers.

I discovered that it does not just work, because for
std::insert_iterator there is an invariant involved that expects the
inserted to be a non empty element of the container, that the element
is inserted into. This invariant is broken with interval containers
because an interval/segment is not an element of an interval
container.

> I shouldn't have to expect to look for the itl specific adder to do this. This
> was the most common usage my applications had for populating interval
> containers using std::copy/transform from my applications existing data
> structures.
>
>> - What is your evaluation of the implementation?
>
> Seems adequate. I was able to port to the portions I needed to CodeWarrior
> 9.4 (from 2003?). IIRC, Joachim integrated the changes(simplifications) that
> allowed itl to work in this environment.

I added all the changes suggested, but I remember that important parts
of the code, itl global operators in particular, still did not compile
with CW. I tried to set up CW myself, but encountered too many
problems and gave up :(

> Performance for the scale of problems I've been dealing with were fine, but
> as has been discussed scaling to larger sizes of millions of items may
> require some specialized containers. I don't see this as a problem, it's
> analogous to std::map and std::unordered_map.

I share this view.

> Hmm, It just occurs to me that for my application I could void the N^2 size
> issue of a split_interval_map<int, set<> > with a non_combining_map<int,
> data> that allowed me to iterate through all items that overlap with an
> arbitrary interval in and ordered fashion. That's not much different from a
> std::map<interval, data>, but with some more interval specific wrapping of
> the upper/lower_bound/equal_range methods.
>
>> - What is your evaluation of the documentation?
>
> I was able to get up and running fairly quickly. As discussed some of the
> colorful terminology got in the way rather than helping. Also I had expected
> to just be able to populate with std::inserter, and had trouble finding docs
> related to this. The example for copy/transform helped, but even during this
> review I have trouble finding the docs for itl::adder/inserter.

True, escaped my notice. Has to be added.

>> - What is your evaluation of the potential usefulness of the library?
>
> Very, I've come across needs for this capability in 3 different jobs in 3
> very different industries. I've had domain specific solutions and was
> looking to create just such a generic library when Joachim submitted his
> library.

That's interesting. I had the basic ideas and first implementation in
1999 / 2000 and I had the immediate impression that it should be made
available to the public domain. But due to other projects and
commercial copyright of the first version, I did not have the
opportunity to make it open source for years. I always thought someone
else would be faster than me ;)

>> - Did you try to use the library?  With what compiler?  Did you have any
>> problems?
>
> Yes, the library is being used in a commercial application built with
> CodeWarrior 9.4 on Mac/Windows and with VC8 and XCode 3.x. The library
> required some modifications to the template declarations for CodeWarrior.

Is it now fully or still only partially compilable with CW?

>> - How much effort did you put into your evaluation? A glance? A quick
>> reading? In-depth study?
>
> I read through the documentation, integrated the library with production
> code, dug through compilation error messages and headers on CodeWarrior to
> port a portion of the library. I've profiled code using itl to understand
> performance implications.
>
>> - Are you knowledgeable about the problem domain?
>
> Yes, I've designed/implemented/optimized integral open-closed Interval and
> IntervalSet classes for use in previous projects.

Thanks again for your review and the support that you provided for the
ITL library.

Cheers,
Joachim