> > <cffunction name="safeString" access="public" hint="strips unsafe > chars" returntype="string" output="false"> > <cfargument name="stringToClean" required="true" type="string" /> > <cfscript> > var uncleanString = arguments.stringToClean; > var cleanString = ""; > // replace "_" and " " with "-" > uncleanString = rereplace(uncleanString,'[_| ]','-','all'); > // remove any chars besaides A-z 0-9 or "-" > uncleanString = > rereplacenocase(uncleanString,'[^a-z|A-Z|0-9|-]','','all'); > // the "+" means "one or more". "---" becomes "-" > uncleanString = rereplace(uncleanString,'[-]+','-','all'); > cleanString = uncleanString; > return cleanString; > </cfscript> > </cffunction>
While I see the point in setting a local variable to the value of the argument (var uncleanString = arguments.stringToClean), the final two lines of the function seem really rather silly: cleanString = uncleanString; return cleanString; I have seen this kind of thing rather a lot in legacy code and it always makes me wonder. What really is the point of the second local variable? Also, if you read it in English, it makes no sense whatsoever. You don't appear to be returning a clean string at all. I think the following is perhaps more sensible if using a local variable is insisted upon (as opposed to just working on the argument directly): var cleanString = arguments.stringToClean; cleanString = rereplace(cleanString ,'[_| ]','-','all'); cleanString = rereplacenocase(cleanString, '[^a-z|A-Z|0-9|-]','','all'); cleanString = rereplace(cleanString ,'[-]+','-','all'); return cleanString; There are times when slimmer code makes less readable code but the inverse can also be true. I think that the extra local variable is more confusing than clarifying. Dominic -- Blog it up: http://fusion.dominicwatson.co.uk ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~| Adobe® ColdFusion® 8 software 8 is the most important and dramatic release to date Get the Free Trial http://ad.doubleclick.net/clk;160198600;22374440;w Archive: http://www.houseoffusion.com/groups/CF-Talk/message.cfm/messageid:301369 Subscription: http://www.houseoffusion.com/groups/CF-Talk/subscribe.cfm Unsubscribe: http://www.houseoffusion.com/cf_lists/unsubscribe.cfm?user=11502.10531.4
