[ic] One-line patch for more_link_template

[Carl Bailey]

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
. . . . . . . . . . . . . . . . . . .