[ic] One-line patch for more_link_template
Carl Bailey
carl at endpoint.com
Mon Oct 19 15:39:07 UTC 2009
On Oct 18, 2009, at 4:59 PM, Peter wrote:
> On 10/18/2009 08:28 AM, Carl Bailey wrote:
>> I've been looking at this for a while now, trying to understand why
>> the above would be better. From what I can tell, there are three
>> ways
>> to call tag_area() in this case:
>>
>> 1) tag_area('scan', "MM=$mv_arg", { form => $form_arg, match_security
>> => 1, }
>> 2) tag_area(undef, undef, { search => "MM=$arg", form => $form_arg,
>> match_security => 1, }
>> 3) tag_area("scan/MM=$mv_arg", undef, { form => $form_arg,
>> match_security => 1, }
>>
>> All three calls return the identical URL.
>>
>> Options 1 and 2 both wind up making a call to escape_scan() to
>> compose
>> the URL. That's necessary in tag_area(), when the routine can not
>> know what search variables will be coming its way. But in the case
>> of
>> sub more_link_template(), the call is always identical, using only a
>> single scan directive, MM. So hard-coding it seems more efficient by
>> eliminating the call to escape_scan(). Since none of the three
>> options above is inherently more readable than the others, I vote for
>> option 3 for the sake of efficiency, but I am ready to be educated if
>> I have missed a reason why one of the other choices is better.
>
> Hard coding things like this is generally not a good idea because it
> removes the ability to change things in the future in one central
> location. In this particular case it means that search parameters
> could
> be stored internally rather than hard-coded into the URL as they are
> now, also they scan specialpage could be changed for whatever reason.
>
>> Replacing "secure => $CGI::secure," with "match_security => 1" is not
>> much of a change. All that happens in vendUrl() is that when it
>> detects match_security, it sets $opt->{secure} = $CGI::secure.
>> However, since using match_security is more self-documenting, I think
>> this is a good enhancement, and will make the change in git-hub.
>
> Again it comes down to hard-coding things which is bad. If it's
> determined at some future time that we want to try to get rid of
> globals
> such as $CGI::secure in favor of object and method calls, then if you
> use match_security the core change to the area tag will cover you, but
> if you use $CGI::secure then that will have to be changed as well.
>
> In general the idea is that you should think twice about hard coding
> things. If you instead stick to using the provided attributes in a
> more
> flexible manner then it helps to prevent problems down the road.
>
>
> Peter
>
> _______________________________________________
> interchange-users mailing list
> interchange-users at icdevgroup.org
> http://www.icdevgroup.org/mailman/listinfo/interchange-users
Not worth arguing further. I have updated github to use Peter's
better version.
http://github.com/carlbailey/interchange/commit/fd129761dabbf4e461538cae96af730bc6db4ca2
Carl
. . . . . . . . . . . . . . . . . . .
Carl Bailey
t: 919-323-8025
carl at endpoint.com
. . . . . . . . . . . . . . . . . . .
More information about the interchange-users
mailing list