Re: [PATCH] StoreID latest implementation in sync with rev 12552. stage 2-3 from 3.

On 01/03/2013 08:55 AM, Eliezer Croitoru wrote:
> On 1/3/2013 5:42 PM, Alex Rousskov wrote:
>> On 01/03/2013 03:16 AM, Eliezer Croitoru wrote:
>>
>>> >Maybe a description like:
>>> >"A copy of StoreID reply for the specific cases which the
>>> >request->store_id needed but not present"

>> Yes, this is better, but still rather convoluted. I suggest something
>> like "Store ID for transactions where the request member is nil",
>> accompanied with an appropriate getter method that uses request when
>> possible and this member when not.
>
> Well the problem is that when accessing the request in these situations
> caused some assertion to crash squid in runtime which I do not event
> want to try getting there.
> I dont have the exact locations right now and will check later but the
> places which this store_id is being used are very small (3-4).
>
> Else then OO design what reasons do we have for using this kind of a
> getter?

One of the reasons is that sooner or later the current or updated code
will crash while "accessing the request in these situations". Just
because you have carefully identified when the request should or should
not be used now, does not mean that your identification will remain
valid a year from now or that nobody will copy your code to a different
location where it will break next week.

This has nothing to do with object orientedness. This is about giving a
correct way of accessing information that may be present in two
different places, one of which may be invalid. If testing request for
being nil is sufficient, this is a simple change. If it is not
sufficient because request may be non-nil but invalid, then it is
probably too dangerous to commit this code and more work is needed to
make it reliable (which may include changing other code, of course).

HTH,

Alex.
Received on Thu Jan 03 2013 - 16:24:11 MST