On 02/04/2014 12:57 PM, Kinkie wrote: > On Sun, Feb 2, 2014 at 10:42 PM, Amos Jeffries <squ...@treenet.co.nz> wrote: >> On 2014-02-03 08:06, Kinkie wrote: >>> >>> Hi, >>> the attached patch (merge from lp:~squid/squid/vector-refactor) is >>> an attempt to refactor Vector and its clients so that: >>> - clients of Vector don't break layering >>> - the Vector API more closely matches std::vector >>> >>> The eventual aim is to replace Vector with std::vector, only if some >>> more accurate measurements (in the lp:~squid/squid/vector-to-stdvector >>> branch) show that this wouldn't cause performance degradations.
> v2 attached. > +Vector<E>::at(unsigned i) > { > assert (size() > i); > return items[i]; > } > > template<class E> > const E & > -Vector<E>::operator [] (unsigned i) const > +Vector<E>::at(unsigned i) const > { > assert (size() > i); > return items[i]; > } Ideally, at() methods should be implemented using [] operators instead of direct access to "items", but that is very minor. > - ErrorDynamicPageInfo *info = ErrorDynamicPages.items[i - > ERR_MAX]; > + ErrorDynamicPageInfo *info = ErrorDynamicPages[i - ERR_MAX]; This non-performance-critical line surrounded by relatively complex index logic should use an at() method instead. Again a minor thing. > - theClient->start (tag + 1, (const char **)attributes.items, > attributes.size() >> 1); > + theClient->start (tag + 1, const_cast<const char > **>(attributes.data()), attributes.size() >> 1); The kind of cast appears to be wrong here: Const_cast<> is normally used to remove const, not add it. It is not used to change the type. Also, .data() is a C++11 method. Should we avoid those for now? To be compatible with older STLs, you may use the address of vector[0] instead AFAICT. All of the above can be addressed during commit IMO. Thank you, Alex.