Re: [PATCH] Bug 3686 - refactor the config options controlling max object size

On 9/02/2013 5:58 p.m., Alex Rousskov wrote:
> On 02/08/2013 05:54 PM, Amos Jeffries wrote:
>>>> - virtual int64_t maxObjectSize() const { return max_objsize; }
>>>> + int64_t maxObjectSize() const;
>>> Please do not remove "virtual". It is a useful reminder that this is a
>>> part of the Store API.
>> Why add virtual table entries? this is not a replaceable function any
>> more. This is an accessor method for the SwapDir base object.
> Store, the SwapDir's parent class has the "virtual" keyword for this
> method. Thus, the method will be virtual in all child classes and will
> be in the virtual table regardless of whether we say "virtual" explicitly:
>
>> Store.h: virtual int64_t maxObjectSize() const = 0;
> The above Store class API gets implemented by these Store children:
>
>> MemStore.h: virtual int64_t maxObjectSize() const;
>> StoreHashIndex.h: virtual int64_t maxObjectSize() const;
>> SwapDir.h: virtual int64_t maxObjectSize() const;
>> SwapDir.h: int64_t maxObjectSize() const;
>
> The reason I believe it is better to say "virtual" explicitly in this
> case is to remind us that maxObjectSize() is a part of the [semi-broken]
> Store API and not some SwapDir "own" method. This will help us when we
> polish the API to remove the common Store root.
>
> Again, being explicit here does not add or remove any virtual table entries.
>
>
> Hope this clarifies,
>
> Alex.
>

That makes sense. Followup patch applied.

Amos
Received on Sat Feb 09 2013 - 05:17:16 MST