Re: [PATCH] refactor Vector

On Tue, Feb 4, 2014 at 2:17 AM, Alex Rousskov
<rousskov_at_measurement-factory.com> wrote:
> On 02/02/2014 02:42 PM, Amos Jeffries wrote:
>
>> in src/HttpHdrRange.cc
>>
>> * This pattern happens several times throughout this patch:
>> - while (!specs.empty())
>> - delete specs.pop_back();
>> + while (!specs.empty()) {
>> + delete specs.back();
>> + specs.pop_back();
>> + }
>>
>> ** std::vector::pop_back() is documented as erasing the element.
>> Squid::Vector does not.
>> - under correct usage of std::vector the loop and delete X.back() would
>> seem to be irrelevant.
>>
>> ** the pattern with loop seems to be equivalent to specs.clear() but far
>> less efficient. Efficiency is saved here only by Squid::Vector not
>> reallocating the items array in pop_back().
>
> An explicit delete is necessary when storing raw pointers to objects.
> Neither std::vector nor Squid's Vector [can] delete objects that
> elements removed by clear() or pop_back() point to.
> A generic container does not know whether it stores "raw pointers to
> objects" or "just objects". When removing elements, it has to use the
> element's default constructor or destructor. When an element is a raw
> pointer, the default constructor and destructor do not do anything
> [useful], so we have to delete those objects by hand using "delete" (or
> use smart pointers instead).

The latter would probably be best, but it's out of scope for this effort IMO.

> One could avoid the explicit loop using std::for_each. However, it would
> not make things more efficient (or more clear) as far as vector memory
> management is concerned.

Maybe it would change something, but it'd be lost in the noise and is
definitely not worth it.
Thanks for the clarification, I had misread the reference.

-- 
    Francesco