Boost logo

Boost :

From: Martin Knoblauch Revuelta (mkrevuelta_at_[hidden])
Date: 2006-10-31 02:59:21


On 10/31/06, Jason Hise <0xchaos_at_[hidden]> wrote:
> I have started looking through the code. It looks like some pretty
> good work thus far. My first comments:
>
> A minor nit: you probably shouldn't be using _STL_ as part of your
> include guards... it makes it look as though the library is already a
> part of the stl (and it isn't... yet ;) )

You are right. Changed.

> Instead of defining your own greater than and less than functors, it
> may be preferable to use greater and less, defined in the <functional>
> header. Just like yours, the stl greater requires Ts to be less than
> comparable rather than greater than comparable.

Right again.

> Your static allocator is probably a bad idea... if someone creates a
> static instance of this container type, the allocator could be
> destroyed before the container destructor is invoked. The container
> destructor would then reference the non-existant object. There are
> ways around this, but IMO it would just be better practice to have
> each container own its allocator.

I see. I didn't think of that case (a static container). Can I
allocate something wit an allocator object, and deallocate it with a
different allocator object of the same class (even after the first one
was destroyed)? For short: are allocators fully static? I mean, the
SGI documentation says they must be, but is it the same in std and
boost? If yes, then I can safely do as you guess.

> You use c-style casts from literals to type W, which should either be
> changed to C++ style casts or explicit constructor calls.

Ok. :)

> It would be good to find a way to rewrite your try-catch blocks in
> terms of helper commit or rollback objects, which release resources in
> their destructors unless cancelled (after a successful operation,
> calling cancel on such an object would be the equivalent of commit).
> This way, the code will still compile if the end user chooses to
> disable exception handling, or if the compiler does not support it.

I'll think how to do this.

> The header is pretty large... breaking it into smaller files, even if
> these aren't publicly exposed, might help readability and
> maintainability.

I agree. It has already become far too large.

> The commenting is good, but nothing is a substitute for formal library
> documentation. At the very least, an html or pdf file should be
> provided. The most important things to cover are which concepts are
> implemented by your container and iterators and what the complexity
> guarantees of all operations are. A discussion of the implementation
> or links to online resources about it would be nice as well, but not
> as vital as the first two items.

I'll work on these. Thanks a lot!

Martin


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