patternMinor
Filtering the attributes for a custom tag
Viewed 0 times
thetagcustomattributesforfiltering
Problem
I have function that takes all of the attributes passed to a custom tag, and returns a select subset. I am try to convert the current function, which does not use closures
Into one that does
Both of these are invoked with
Is this the right approach? Should arguments passed to the closure functions be scoped?
string function passThrough(required struct attr) output="false" {
local.result = "";
for(local.myKey in arguments.attr) {
if (variables.myKey.left(5) == "data-" || variables.myKey.left(2) == "on" || variables.myKey.left(3) == "ng-") {
local.result &= ' #local.myKey.lcase()#="#arguments.attr[local.myKey].encodeForHTMLAttribute()#"';
} // end if
} // end for
return local.result;
}Into one that does
string function passThrough(required struct attr) output="false" {
arguments.attr.filter(
function(key, value) {
return (key.left(5) == "data-" || key.left(2) == "on" || key.left(3) == "ng-");
}
).each(
function(key, value) {
local.result &= ' #key.lcase()#="#value.encodeForHTMLAttribute()#"';
}
);
return local.result;
}Both of these are invoked with
... invoke("bootstrap", "passThrough", {attr = attributes});Is this the right approach? Should arguments passed to the closure functions be scoped?
Solution
From a code review perspective, I'd say:
Overall, I find the example without closures more readable, but the feedback above would make it more so IMO. Here's how it could look with closures:
http://trycf.com/scratch-pad/gist/d6ce9527fa3294b286cc
- The function, "passthrough", is doing two things and doesn't describe what it does very clearly. I'd suggest two functions here, "filterAttributes" and "structToQueryString" (or whatever their exact purpose is)
- The long filter logic could be concisely expressed as a regular expression to avoid the long line of logic ("^((data|ng)-|on)")
- As for scoping closure arguments, I'd suggest this was a matter of preference. If the coding guidelines you work with ask for all variables to be scoped, then yes, they should be scoped. I personally find it more readable to not have them. Having small and concise functions should help prevent scoping issues.
Overall, I find the example without closures more readable, but the feedback above would make it more so IMO. Here's how it could look with closures:
struct function stripNonDataNgAndJsAttributes( required struct attributes ) {
var patternToKeep = "^((data|ng)\-|on)\S";
return attributes.filter( function( key, value ){
return key.findNoCase( patternToKeep );
} );
}
string function structToQueryString( required struct input ) {
return input.reduce( function( result, key, value ){
return result & ' #key.lCase()#="#value.encodeForHTMLAttribute()#"';
}, "" );
}http://trycf.com/scratch-pad/gist/d6ce9527fa3294b286cc
Code Snippets
struct function stripNonDataNgAndJsAttributes( required struct attributes ) {
var patternToKeep = "^((data|ng)\-|on)\S";
return attributes.filter( function( key, value ){
return key.findNoCase( patternToKeep );
} );
}
string function structToQueryString( required struct input ) {
return input.reduce( function( result, key, value ){
return result & ' #key.lCase()#="#value.encodeForHTMLAttribute()#"';
}, "" );
}Context
StackExchange Code Review Q#121963, answer score: 5
Revisions (0)
No revisions yet.