Hi Daniel, Daniel James wrote:
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.
Thanks. Maybe the use of Intrusive to model relationships can be converted in a new Boost library.
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.
Ok. Something like a "quick guide for the impatient".
It's specified that islist and ilist are implemented as circular lists - this is an implementation detail and doesn't need to specified.
Ok. But I'm thinking that maybe list_algorithms and slist_algorithms should be called circular_list_algorithms... because maybe in the future algorithms for null ended list could be added. Just a thought.
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.
Well, i suppose that your "smart pointer" will have 3 pointers, one being the actual pointer to the pointee and the other two to form the list. You can use list_algorithms to do what you describe (add a node to a group of nodes and remove a node from a group of nodes without having a reference to the group. You just need to define your NodeTraits compatible with these algorithms. Intrusive could also define a simple node and its NodeTraits class, so that you can use the algorithms directly without defining anything.
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.
Ok. Already mentioned and I agree that it's more self-describing.
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" prefix will disappear, don't worry
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).
Ok. Another place for improvements.
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;
Well, you save the "::value_traits" part so I'm not sure if that's a big improvement.
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.
I will think about this. Maybe the common cases should be simpler, but for every container you need to specify the value type and the hook the container will use. For member hooks there is no default possibility, because the offset of the member can be unknown. With base hooks, maybe the situation can be simpler, because the transformation between the hook and the value type is based on static_cast.
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.
Ok. A default tag sounds good.
Daniel
Regards, Ion