On 2011-04-28 14:05, Roy Lyseng wrote:
> Hi Tor, Olav,
>
> like Olav, I do like this patch. I do however have a few comments
> which I hope you will consider:
>
> 1. The mem_root member.
>
> I suggest that this member be made const, because I cannot see a
> requirement to change mem_root during the lifetime of an object. In
> fact, I think that we may be in for some nasty surprise if we do, and
> it will at least constrain the way the mem_root allocator can be
> implemented.
>
> It means that mem_root must be set in the constructor (I think that is
> good), and you can get rid of the set_mem_root() function (really good).
done
>
> 2. Initial size, and lazy allocation
>
> If you take initsize as part of the template, you do not need to have
> reserve() as an externally visible function, the initial space will be
> set when the object is constructed. It may also be used to delay
> allocation until the first insertion, which is slightly more efficient
> for objects with zero final size.
>
For this case, we want initial size == 0, and only allocate memory if we
actually need it.
I guess you are asking for "the size of the initial allocation if the
initial size == 0"
I would prefer to keep the interface/semantics as close to std::vector
as possible,
rather than introducing 'initial size' which really means 'initial size
if we insert anything into the array'.
> 3. The name.
>
> I think that we should be able to come up with a better name than
> Mem_root_array for the new class. The name should better reflect the
> functionality that it provides, rather than being hung up with the
> implementation detail of the mem_root. There may also after a while be
> a whole set of such classes that rely on a dedicated memory allocator,
> so they might need a good family name. Perhaps a namespace would we
> good for this?
>
> Unfortunately I do not have a good name suggestion ;)
namespace memroot
{
class Array...
}
??
>
> Thanks,
> Roy
