
Hello, Sorry, this is review is a bit late and rushed. I feel like I'm back at college finishing a paper the night after it's due... I've read through the documentation and looked at some of the implementation (mostly the unordered containers). I think Intrusive should certainly be added to Boost. The design seems very flexible and anticipates most of the use cases that I can think of. Intrusive data structures can be useful. As well as the potential memory, performance and exception safety gains, they are perhaps a more natural way to model the relationships between entity types than containers of pointers. The documentation is good. But I think it could be improved if it showed complete code examples much sooner - at the moment it goes into the concepts in some depth before the reader gets a feel for how they fit together. I think it's possible to show how to make basic use of the simpler intrusive containers and then ease the reader into the more involved details. It's specified that islist and ilist are implemented as circular lists - this is an implementation detail and doesn't need to specified. It might also be useful to have intrusive structures without container types. Say you were implementing a smart pointer where all the pointers to the same object are linked together by a circular list. The copy constructor has a pointer would add the new pointer to list, the assignment operator would remove the pointer from its current list, and add it to the new one. All of this would be done without ever referring to a container. And it's not clear who'd be responsible for the container. This would require the hooks to be more complicated (I guess they would be something more than just hooks). And they'd probably need to use the curiously recursive template pattern. But maybe this is outside of the scope of the library. I think this may have already been mentioned by someone else, but the true/false template parameter (for constant time size) isn't very clear. In such cases, I always forget which is true, and which is false - and that's if I can remember what the parameter stands for in the first place. I think it'd be clearer to use a couple of appropriately named types. And I agree with other people's dislike of the 'i' prefix - it's already used in the string algorithms for 'case-insensitive'. Also ilist and islist look a little too similar. I had a quick look at the tests and it looks like there could be some more (which is of course always true). Exception tests could be useful where needed (Boost.Test has a pretty good exception testing framework, but it's not documented). Finally, something which I haven't spent enough time on. And it's late. So I'm probably saying something really stupid. But basic containers seem a little too complicated to define. Especially for dependant types inside templates. Something like: typedef islist< islist_base_hook<my_tag>::value_traits<MyClass> > BaseList; Could perhaps be: typedef islist<MyClass, islist_base_hook<my_tag> > BaseList; With a third parameter that could used to specify an alternative value_traits when needed. Which I think fits in better with the typical STL way of doing things. It might be possible to do something simpler than this for some special (but common) cases - but the only ones I've thought of so far are problematic. It does seem odd to have to sepecify 'islist_base_hook' for 'islist' - I'd expect that to be the default. I guess to elide that there would need to be some mechanism to distinguish between tags and hooks - allowing you to use just the tag. A bit iffy. Also, a default tag would be useful. Most of the time a container will only have a single hook of a certain type and declaring a tag in this case is unnecessarily verbose. Daniel