Best practice for scoped variables in a closure function

What is the best practice for reverencing the argument variables within a closure function? Documentation doesn’t show scopeing, so in the case of a structure it says:

someStruct = {a=1,b=2,c=3};

structEach(someStruct,function(key,value) {
     writeOutput('Key ' & key & ' is ' & value & '; ');
});

To avoid scope searching, potentially referencing the wrong scope/variable, and maintain thread safety, shouldn’t we explicitly use the arguments scope?

Bonus question: Does using the arguments scope in this example affect thread safety or are closure function arguments already thread safe?

someStruct = {a=1,b=2,c=3};

structEach(someStruct,function(key,value) {
     writeOutput('Key ' & arguments.key & ' is ' & arguments.value & '; ');
});

The docs don’t specifically mention it, I imagine, because there’s no specific guidance particular to closures and scoping of its arguments. If you want: go ahead. Same with any other sort of function.

The “difference” with function expressions is that they can be declared within other functions, so within the body of the function expression, the arguments and local scopes refer explicitly to its own scopes, and specifically not the same scopes from any enclosing function. EG:

function f(x, y){
    var a = 1
    var g = function (x) {
        writeOutput(x)// g's one
        writeOutput(arguments.x) // still g's one
        // f's one is not accessible at all as it's shadowed by g's one

        writeOutput(y) // f's one
        writeOutput(arguments.y) // error, there is no `y` in g's arguments scope

        writeOutput(a) // f's one
        writeOutput(local.a) // error, there is no `a` in g's local scope
    }
}

Best practice with scoping remains “don’t scope unless one has to”, IE: to be precise and clear when referencing variables outside the order of the look-up chain.

There is not a need to explicitly scope arguments or local variables in well-written code.


Adam

1 Like

@AdamCameron

Just assking, do you mean generally or specifically within closures?

Any code.

Adam, thank you for the very detailed answer. The larger best practice of not scoping unless you have to is news to me. What I’ve read about the look-up chain concept implied (to me) that because it can be avoided by explicitly scoping it should be because of the savings from not having to invoke this extra processing. Do I not understand that correctly?

I get that its a convenience thing, and that my code would be more svelte without all of the extra variable names littered throughout, but is there a real performance/safety/other reason to not always scope? Is this still your gospel on the matter? Adam Cameron's Dev Blog: CFML survey results: scoping of variables-scope variables

Don’t worry about the computer, it’ll take care of itself. Code is for humans: make it as clear as possible for humans. If your code in a method is so complicated that one might think it’ll be made clearer by scoping local variables… it’s not the lack of scoping that’s yer problem.

The scope-lookup overhead is well into the realms of micro- (and premature ~) optimisation. Deal with that as a tuning tactic once all things that will actually deliver real improvements have been exhausted.

Similarly, if you’re using variables that require being scoped (request, form, etc), then you’ve probably designed yer code wrong. One should not be accessing scopes like that in one’s method implementations. Well: other than the ones that are explicitly designed to transfer them into a request context object, or something like that. But the framework should be taking care of that already.

I’d never trust anything that guy wrote.

In all seriousness though, my bottom line then seemed to be:

I’m not sure. For new code I don’t see the need to be slavish about this. For legacy code that’s bloated, it could be beneficial if nothing can be done about the bloat.

In the codebase I’m working on ATM the scoping practices are… “lacking uniformity” shall I say. We’ve adopted a “scope everything” policy, because unfortunately the code is legacy-quality, and given it’s untested (/able) and we’ll be phasing it out soonish, we’re sucking it up and using variables scoping as lipstick. As it were.


I think people’s policies of “scope everything” is usually borne out of having codebases that are not as well designed as they could be, so there’s often methods that do a bunch of different things over dozens and dozens of lines of code. If that’s your codebase: maybe do that.

That was a very long-winded “it depends”, eh? :wink:

2 Likes

where possible, it’s best to use strict scope cascading and disable searching query result sets (for both performance and security) see settings → scope in the admin

unless you are on a nice new code base and can use full null support, not scoping values where a null value can be encountered, can lead to a scope bypass ( a variable with a null value in cfml doesn’t exist and will be skipped when scope cascading ) which then may let an attacker pass in an overriding value for say userId from the url scope

with functions we have both the arguments and local scope, which we contemplating allowing them to be optional merged in the future, so that the argument scoping isn’t required (local scope is the first scope in a function, arguments is the secondary)

there’s also local mode.

but explicitly referencing outer scopes in this context is a somewhat unresolved aspect of cfml to date

1 Like

Maybe I am misunderstanding you here. I wrote some code:

metadata = getApplicationMetadata()
writeDump(var=[
    "localMode: " = metadata?.localMode,
    "nullSupport: " = metadata?.nullSupport,
    "enableNullSupport: " = metadata?.enableNullSupport,
    "scopeCascading: " = metadata?.scopeCascading,
    "searchImplicitScopes: " = metadata?.searchImplicitScopes
], format="text")

function setToNull(){}

variables.someVar = "Don't ruin it"

function f() {
    local.someVar = setToNull()

    someVar = "I told you not to ruin it FFS"
}
f()
writeDump(var=[
    "variables.someVar after f(): " = variables.someVar
], format="text")

Output:

Struct (ordered)
localMode:  boolean false
nullSupport:  boolean false
enableNullSupport:  boolean false
scopeCascading:  string standard
searchImplicitScopes:  boolean true

Struct (ordered)
variables.someVar after f():  string Don't ruin it

It doesn’t - to me - look like “a variable with a null value in cfml doesn’t exist and will be skipped when scope cascading”…?

FWIW: same behaviour in CF, and this is also what I’d expect.

I’ve not bothered with a trycf link to this because it’s not reliable when it comes to checking scope behaviour, cos it monkeys with the code before running it, and doesn’t run it in its own context. So you’ll need to run this on a proper Lucee/CF situation.