Mailing List Archive

Re: [interchange] Fix code which assumes we already have a hashref
On 08/11/16 08:32, David Christensen wrote:
> Fix code which assumes we already have a hashref
> + $item = { 'code' => $item } unless ref $item;
> +
> return $item->{mv_cache_price}
> if ! $quantity and ref($item) and defined $item->{mv_cache_price};
>
> - $item = { 'code' => $item } unless ref $item;

This does not look like it was broken before, it certainly did not
assume that we had a hashref as it specifically tested for it in the if
statement.


Peter

_______________________________________________
interchange-users mailing list
interchange-users@icdevgroup.org
http://www.icdevgroup.org/mailman/listinfo/interchange-users
Re: [interchange] Fix code which assumes we already have a hashref [ In reply to ]
> On Nov 7, 2016, at 9:44 PM, Peter <peter@pajamian.dhs.org> wrote:
>
> On 08/11/16 08:32, David Christensen wrote:
>> Fix code which assumes we already have a hashref
>> + $item = { 'code' => $item } unless ref $item;
>> +
>> return $item->{mv_cache_price}
>> if ! $quantity and ref($item) and defined $item->{mv_cache_price};
>>
>> - $item = { 'code' => $item } unless ref $item;
>
> This does not look like it was broken before, it certainly did not
> assume that we had a hashref as it specifically tested for it in the if
> statement.

True, I think I missed it ref() in that condition; had some across an error in an installation related to dereferencing a string and thought I’d found the solution. I could revert or just leave it, as it doesn’t have any practical difference; otherwise it’s just VCS noise at this point.
--
David Christensen
End Point Corporation
david@endpoint.com
785-727-1171




_______________________________________________
interchange-users mailing list
interchange-users@icdevgroup.org
http://www.icdevgroup.org/mailman/listinfo/interchange-users
Re: [interchange] Fix code which assumes we already have a hashref [ In reply to ]
On Fri, 11 Nov 2016, David Christensen wrote:

>> On 08/11/16 08:32, David Christensen wrote:
>>> Fix code which assumes we already have a hashref
>>> + $item = { 'code' => $item } unless ref $item;
>>> +
>>> return $item->{mv_cache_price}
>>> if ! $quantity and ref($item) and defined $item->{mv_cache_price};
>>>
>>> - $item = { 'code' => $item } unless ref $item;
>>
>> This does not look like it was broken before, it certainly did not
>> assume that we had a hashref as it specifically tested for it in the if
>> statement.
>
> True, I think I missed it ref() in that condition; had some across an
> error in an installation related to dereferencing a string and thought
> I’d found the solution. I could revert or just leave it, as it doesn’t
> have any practical difference; otherwise it’s just VCS noise at this
> point.

If you want to leave it the new way, I think we should remove this now
redundant check:

and ref($item)

so that at least the net result of the version control churn is one less
check of ref($item) in that same area. :)

Jon


--
Jon Jensen
End Point Corporation
https://www.endpoint.com/
Re: [interchange] Fix code which assumes we already have a hashref [ In reply to ]
Here we see some of my thinking at the time when I was working very hard to
save cycles in places that are iterative. The simple check for a scalar,
instead of the 6 times more expensive hash member, was done before taking
the time to instantiate the hash. I believe that some of these things,
mostly resisting objects in code likely to be called hundreds of times per
page, kept Interchange fast enough to be competitive.

On Fri, Nov 11, 2016 at 3:15 PM, Jon Jensen <jon@endpoint.com> wrote:

> On Fri, 11 Nov 2016, David Christensen wrote:
>
> On 08/11/16 08:32, David Christensen wrote:
>>>
>>>> Fix code which assumes we already have a hashref
>>>> + $item = { 'code' => $item } unless ref $item;
>>>> +
>>>> return $item->{mv_cache_price}
>>>> if ! $quantity and ref($item) and defined
>>>> $item->{mv_cache_price};
>>>>
>>>> - $item = { 'code' => $item } unless ref $item;
>>>>
>>>
>>> This does not look like it was broken before, it certainly did not
>>> assume that we had a hashref as it specifically tested for it in the if
>>> statement.
>>>
>>
>> True, I think I missed it ref() in that condition; had some across an
>> error in an installation related to dereferencing a string and thought I’d
>> found the solution. I could revert or just leave it, as it doesn’t have
>> any practical difference; otherwise it’s just VCS noise at this point.
>>
>
> If you want to leave it the new way, I think we should remove this now
> redundant check:
>
> and ref($item)
>
> so that at least the net result of the version control churn is one less
> check of ref($item) in that same area. :)
>
> Jon
>
>
> --
> Jon Jensen
> End Point Corporation
> https://www.endpoint.com/
> _______________________________________________
> interchange-users mailing list
> interchange-users@icdevgroup.org
> http://www.icdevgroup.org/mailman/listinfo/interchange-users
>
>


--
The problem with Internet quotations is that many of them
are not genuine. -- Abraham Lincoln
Re: [interchange] Fix code which assumes we already have a hashref [ In reply to ]
On 12/11/16 11:43, Mike Heins wrote:
> Here we see some of my thinking at the time when I was working very hard
> to save cycles in places that are iterative. The simple check for a
> scalar, instead of the 6 times more expensive hash member, was done
> before taking the time to instantiate the hash. I believe that some of
> these things, mostly resisting objects in code likely to be called
> hundreds of times per page, kept Interchange fast enough to be competitive.

With this in mind the recent chagne should be reverted.

> True, I think I missed it ref() in that condition; had some
> across an error in an installation related to dereferencing a
> string and thought I’d found the solution.

The check for ref($item) was added by Jon only this past April. I would
say the install you were having problems with is likely older than that.
Simply updating would have fixed it.

> I could revert or
> just leave it, as it doesn’t have any practical difference;

It does, as Mike points out above.


Peter

_______________________________________________
interchange-users mailing list
interchange-users@icdevgroup.org
http://www.icdevgroup.org/mailman/listinfo/interchange-users