[boost] Re: Formal Review: Circular Buffer

From: Thorsten Ottosen (nesotto_at_[hidden])
Date: 2004-03-05 01:31:04

Hi Jan,

This is not my final review, but just some comment and questions.
It's based solely on my reading of the docs.

1. your rationale mentions many characteristics that I would expect of
std::deque. Do you have any idea
   how big the performance difference is?

2. You mention "Guarantee of basic exception safety" as a design criteria. I
would expect many operations
    to have a stronger guarantee.

3. you write "In fact the circular_buffer is defined in the file
boost/circular_buffer/base.hpp, but it is necessary to ". I don't see
   why the user should be bothered with this implementation detail.

4. Type Requirements: don't T need to be Assignable too?

5. you could remove one version of push_back and push_front by using default
arguments. Similar for insert()/rinsert()

6. What's the use of "return_value_type" ?return_value_type  operator[]
(size_type index) const
        Return the element at the index position.
  Should it not be const_reference?

7. when you state contracts like
Precondition:
*(this).size() > index
don't you mean (*this).size() ? I would prefer that you omitted this
entirely.

8. maybe you should add const_pointer data() const ?

9 I assume you provide the basic exception safety for set_capacity().
However, I think you could achieve the strong guarantee
   by (1) allocating the new buffer and (2) start to copy elements [might
throw] (3) swap the two buffers upon completion. The same goes for
   resize(). Why are they both provided? (If you made resizes 2nd argument
to be the bool, then how would they differ?]

10.  Can't the exception guarantee for the copy-constructor be stronger too?
(and assign(), and push_back(), push_front(), insert() )

11. Some operations must be nothrow: swap(), size(), capaciity(),
pop_back(), pop_front().

Besides that, I think you documentation is excellent.

br

Thorsten