Boost logo

Boost :

From: Dave Harris (brangdon_at_[hidden])
Date: 2007-03-20 09:19:39


In-Reply-To: <45F5011E.8484B515_at_[hidden]>
joaquin_at_[hidden] wrote (abridged):
> The review of Ion Gaztañagas's Intrusive library begins today

I have spent half an hour reading the documentation. Overall I like it. I
have some questions and comments.

Is there any reason why Destroyers must destroy? It seems to me you could
have a Destroyer which pushed the value onto another list. Is that right?
If so, is it a reasonable thing to do? If so, perhaps "Destroyer" is not
the best name. I'd suggest "Sink" instead.

Could remove_and_destroy take a default Destroyer argument that deleted
the value?

I hate the name "current" for the function that turns a value into an
iterator. I'd prefer "to_iterator" or "as_iterator" or similar. "Current"
just doesn't seem relevant to what it does, and the core thing it does is
not indicated by that name.

Did you consider having pop_back() return a reference to the value popped?
As I understand it, the usual reasons why this a bad thing don't apply
here; returning a reference cannot throw, and the value referred to isn't
deleted.

Did you consider making it easier to use as a replacement for list<value*>
? The semantics are already close, but the intrusive lists use references
where the non-intrusive ones use pointers. I can see me having to edit out
a lot of asterixes in my existing code.

I was going to ask whether it would have been a good idea to allow a value
type to be in several lists of the same type, but the more I think about
it the more I agree that requiring the lists to be different types is
best.

I agree with Tom Brinkman that the introduction page of the documentation
could do more to motivate the library. The core idea of using memory in
the value type to store container-specific data, which is what makes the
container "intrusive", needs to be mentioned earlier.

I was pleased to see you mention the typical size overheads for values and
containers. It seemed to me that the overhead for containers was a bit
buried. I'd recommend you draw up a little comparison table which listed
typical overheads for each type of container - hopefully including a good
implementation of std containers if that's not too controversial.

I would also like to see some speed measurements comparing std with
intrusive lists and vectors. Obviously user's milage will vary, but given
that performance is a major motivation for using this library I'd like to
see at least some empirical indication that it can be faster.

Regardless of the above I vote that Intrusive be accepted as a boost
library.

I am fairly knowledgable of the domain, having spent a lot of time using
intrusive lists in C 20 years ago. My current app has some rigid
data-structures currently based on std::list which could probably benefit
from intrusive lists, but I'm not currently in a position to try this so
the above is based only on reading the online documentation.

> To: boost_at_[hidden], boost-users_at_[hidden],
> boost-announce_at_[hidden]
> Reply-To: boost-announce_at_[hidden]

Incidently, I tried to send this message by replying, and my mail software
used the Reply-To: field and sent it to boost-announce, where it was
(unsurprisingly) rejected. Maybe Reply-To: could be set to
boost_at_[hidden] instead?

-- Dave Harris, Nottingham, UK.


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk