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@hotmail.com>:
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