[ic] One-line patch for more_link_template

[Peter]

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