Hi Pavel! Thank you very much for your comments. I agree with most of them. Thanks also for the picture. Here are my notes to some of your comments.
> > Few notes to latest source: > > 1. circular_buffer_adaptor.html: the link in > > "The circular_buffer_space_optimized is defined in > the file boost/circular_buffer.hpp." > > points to wrong source file. I just want the user to include the circular_buffer.hpp regardless if she uses either circular_buffer or its adaptor. Maybe I should write that the circular_buffer is defined in circular_buffe.hpp but the link will point to circular_buffer_base.hpp. Similarly for the circular_buffer_space_optimized. > > 5. Borland C++ Builder 6.4 doesn't allow to bring > operator[] via using. > > This is workaround in circular_buffer_adaptor.hpp: > > #include <boost/detail/workaround.hpp> > > ... > > #if BOOST_WORKAROUND(__BORLANDC__, BOOST_TESTED_AT(0x564) ) > reference operator [] (difference_type n) { circular_buffer<T, > Alloc>::operator[](n); } > const reference operator [] (difference_type n) const { > circular_buffer<T, Alloc>::operator[](n); } > #else > using circular_buffer<T, Alloc>::operator[]; > #endif > What about just overriding operator[] for all compilers? (With the note that some compilers doesn't support using for operators.) > Also the "<< 1" and ">> 1" can be in this case safely replaced by * 2 and / > 2 > (it is unsigned). > What is wrong with "<<1" and ">>1". I think it is safe and it is more effective for some compilers which don't optimize the *2 operation. > > Shrink algorithm can be: > a. allow hysteresis when shrinking > if new_size < current_capacity / 3 then shrink_to(std::max(min_capacity, > current_capacity / 2)) Why current_capacity / 3 ? > > The constructors and set_capacity() would need one more paramerer or > overload, > capacity() could return pair<>. clear() would need change. Maybe new method can be introduced e.g. min_capacity(). The capacity method stays unchanged. > > > 8. circular_buffer_space_optimized<> constructor with circular_buffer<> > parameter can be provided, similarly operators <, >, ==, !=, > and possibly swap() (swaping only items). > No, I don't want to go this way. Suppose some new circular_buffer adaptor with new features will be introduced. Then you will have to provide operators for the base circular_buffer and the circular_buffer_space_optimized. If you add another adaptor the number of functions will rise exponencially. If you look e.g. on std::stack it also doesn't provide operators for std::deque. > > > 9. circular_buffer_base.hpp and circular_buffer_adaptor.hpp: > instead of check > > #if !defined(BOOST_NO_FUNCTION_TEMPLATE_ORDERING) || defined(BOOST_MSVC) > > is enough to use > > #if !defined(BOOST_NO_FUNCTION_TEMPLATE_ORDERING) > > (it is defined for MSVC). I don't know why, but this does not work. You have to include "|| defined(BOOST_MSVC)" too. > > 14. Btw, isn't cb_iterator::operator[]() added by mistake? > I have never seen such an operation for iterator. > No, iterators do have this operator. > > > 16. RAII vs try blocks: > > > > 1. IMO the macro based exception handling isn't needed, it is better > > > to use RAII, like: > > > ... > > > > > > can be replaced by: > > > > > > void set_capacity(size_type new_capacity) { > > > if (new_capacity == capacity()) return; > > > pointer buff = allocate(new_capacity); > > > > > > struct deleter_t { > > > pointer data; > > > size_type capacity; > > > deleter_t(pointer p, size_type s) : data(p), capacity(s) {} > > > ~deleter_t() { deallocate(data, capacity); } > > > }; > > > deleter_t guard(buff, new_capacity); > > > > > > size_type new_size = new_capacity < size() ? new_capacity : size(); > > > std::uninitialized_copy(end() - new_size, end(), buff); > > > destroy(); > > > m_size = new_size; > > > m_buff = m_first = buff; > > > m_end = m_buff + new_capacity; > > > m_last = full() ? m_buff : m_buff + size(); > > > guard.data = 0; > > > } > > > > > I think this is not a good idea because > > - it won't work - the ~deleter_t() destructor does not have access to > > the deallocate() method > > - the original macros are more explanatory (it is easier to understand) > > - every STL implementation is using such macros > > > It may be possible to pass member function pointer or make deleter friend. > > (Another possibility would be to use Alexandrescu's ScopeGuard.) > > There's one good reason to avoid catch(...): on Win32 it interferes with > system exceptions handling. If someone uses __try/__catch/__finally, > these won't be called after throw; and also it won't be possible > to fix and continue system exception. > > If I remember right, Digital Unix compiler had similar option for signals. My colleague told me, if such a system exception ocurrs the destructor of the exception guard won't be called. Check this out please. He also told me that the system exceptions have nothing to do with C++ exceptions except that catch(...) catches them. Btw. I would wonder if all the STL implementations would be just wrong. > > 24. Question: do you plan to add Mojo to circular_buffer? I don't know yet. I have to admit that I read the article about the mojo technique just briefly. I will let you know when I decide. Jano -- Jan Gaspar | [EMAIL PROTECTED] Whitestein Technologies | www.whitestein.com Panenska 28 | SK-81103 Bratislava | Slovak Republic Tel +421(2)5930-0735 | Fax +421(2)5443-5512 _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost