Hi internals,
In PHP, variables are currently scoped to the function scope, and can't be scoped to a block scope. This makes it difficult to reason about how a variable will actually be used at a glance, especially in long functions, or top-level statement lists of a file.
(or how the variable was originally intended to be used)
The function scope lifetime of variables in PHP is similar to how JavaScript treated variables with var
, before the introduction of the let
statement in JS:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/let
Would there be any interest in adding something similar to this in PHP?
(Or have similar ideas been proposed in the past? I didn't see any.)
Correctness would be enforced as a best-effort at compile-time - the variables would continue to only be freed at the end of the function call.
(e.g. __destruct() only gets called when the variable gets reassigned or the function call ends)
- Freeing it immediately would likely require the equivalent of a try{} finally{} to handle exceptions.
try statements prevent opcache from optimizing a function, the last time I checked.
{
let $myObject = new MyClass(), $flag = true;
$myObject->process($flag);
}
// Accessing or setting $myObject or $flag outside a different let is an `E_COMPILE_ERROR` // because it was declared anywhere else in the function body
// as a let statement
$myObject->doSomething();
// `E_COMPILE_ERROR` to access $key or $value after, outside of a separate `let` scope
foreach ($arr as let $key => let $value) {
// It would be a compile error to declare a gotoLabel: inside of a scope of a let statement.
}
// `E_COMPILE_ERROR` to access $key or $value after
echo $$var; // `E_COMPILE_ERROR` to use variable variables in the same scope, to enforce this as much as possible.
{
let $outer = 1;
{
let $outer = 2; // `E_COMPILE_ERROR` to declare in a different scope, can only reassign
$outer = 3; // this is allowed
}
}
{
let $myRef =&$other;
}
{
let $myRef = true; // This removes myRef from the reference group before assigning a new value to myRef - $other is unmodified.
}
I say "best-effort" because require() and $GLOBALS and get_defined_variables() can still read or modify or make references to variables outside of a let
,
depending on what's in the function body or top-level statements.
- Tyson
Am 15.03.2020 um 17:47 schrieb tyson andre tysonandre775@hotmail.com:
{
let $myObject = new MyClass(), $flag = true;
$myObject->process($flag);
}
// Accessing or setting $myObject or $flag outside a different let is anE_COMPILE_ERROR
// because it was declared anywhere else in the function body
// as a let statement
$myObject->doSomething();
What you are suggesting is that a let statement would switch PHP to an altogether different mode where function-scoped and explicit global variables were suddenly disallowed and an error within that function (or only after the let?). What about variables not declared with let, would they still function normally?
That sounds like a bad idea to me as I'd have to look at the whole function to see if there is a let somewhere to determine the function's/variable's mode.
I think that 'let' would only be feasible in a similar fashion to JavaScript: The lexically scoped variable disappears as if the whole block was not there.
How complicated something like that would be to implement I cannot judge and I haven't spent a lot of time thinking about other problems of this proposal.
- Chris
What you are suggesting is that a let statement would switch PHP to an altogether different mode where function-scoped and explicit global variables were suddenly disallowed and an error within that function (or only after the let?).
No, the variables with the same name as the variables in any "let" in that function body would be disallowed. In this proposal, variables, parameters, and closure uses with different names can be declared and used normally.
I mentioned $GLOBALS as one of the ways in which this proposal is a best-effort. However, I'm not proposing to ban $GLOBALS (but to ban $$var).
- The top-level statements of the file can either be the global variables or the variables in the function which required the file. It's not known which at compile time, so I don't plan to ban $GLOBALS.
That sounds like a bad idea to me as I'd have to look at the whole function to see if there is a let somewhere to determine the function's/variable's mode.
A developer would run php --syntax-check
to know if the variable is being accessed incorrectly in a file.
Various tools or scripts exist to do that, in Continuous Integration or in editors/IDEs.
If you meant something else, could you give a code example?
- Tyson
Hi Tyson,
I think this is an interesting idea, but the way you describe it does
seem to introduce a lot of caveats and additional restrictions.
For instance, this feels very odd to me:
Correctness would be enforced as a best-effort at compile-time - the variables would continue to only be freed at the end of the function call.
Intuitively, I would expect this:
{
let $x = new Foo;
$x->bar();
}
somethingElse();
to be equivalent to this:
let $x = new Foo;
$x->bar();
unset($x);
somethingElse();
Could you explain a bit more what the difficulties are in implementing
it this way?
Regards,
--
Rowan Tommins (né Collins)
[IMSoP]
Intuitively, I would expect this:
{
let $x = new Foo;
$x->bar();
}
somethingElse();to be equivalent to this:
let $x = new Foo;
$x->bar();
unset($x);
somethingElse();
If this feature freed variables when going out of scope,
it would be compiled more like the following (if there were multiple lets):
JavaScript doesn't have destructors, but PHP does, which makes the implementation a tiny bit more complex.
try {
$x = new Foo();
$otherLet = new Other();
$x->bar();
} finally {
// try/finally is useful if unset($otherLet) could throw from __destruct
try {
unset($otherLet);
} finally {
unset($x);
}
}
somethingElse();
Actually, thinking about this again, performance is hopefully less of a concern.
I could safely avoid using try/finally
if this was outside of a try
block,
as long as this was in a function scope instead of a global scope.
(All variable go out of scope for uncaught exceptions)
This idea is still a work in progress.
It would definitely be useful to free immediately for objects (or arrays with objects) which call __destruct
when the reference count goes to 0, such as RAII patterns.
My concern about adding an extra unset()
is that it would add some performance overhead,
discouraging using this feature, but opcache should be able to optimize the unset out when unnecessary.
I touched on the reasons for avoiding try
blocks earlier, but hopefully they're not needed.
The global scope is less likely to be vital to performance in most cases.
- Freeing it immediately would likely require the equivalent of a try{} finally{} to handle exceptions.
try statements prevent opcache from optimizing a function, the last time I checked.
If this feature freed variables when going out of scope,
it would be compiled more like the following (if there were multiple lets):JavaScript doesn't have destructors, but PHP does, which makes the implementation a tiny bit more complex.
try { $x = new Foo(); $otherLet = new Other(); $x->bar(); } finally { // try/finally is useful if unset($otherLet) could throw from __destruct try { unset($otherLet); } finally { unset($x); } } somethingElse();
I'm still not 100% clear why all this would be necessary. Do you know
how the equivalent code works with function-scoped variables? As far as
I can see, returning from a function will successfully call multiple
destructors even if one of them throws an exception. Could exiting block
scope use that same algorithm?
Having the variable become inaccessible but not actually deallocated
seems like it would cause a lot of confusion.
For instance:
{
let $fh1 = fopen('/tmp/foo', 'wb');
flock($fh1);
fwrite($fh1, 'Hello World');
// no fclose()
, but $fh1 has fallen out of scope, which would
normally close it
}
{
let $fh2 = fopen('/tmp/foo', 'wb');
flock($fh2, LOCK_EX); // won't obtain lock, because $fh1 is still
open, but no longer accessible
}
Regards,
--
Rowan Tommins (né Collins)
[IMSoP]
Hi Rowan,
Do you know how the equivalent code works with function-scoped variables?
As far as I can see, returning from a function will successfully call multiple
destructors even if one of them throws an exception. Could exiting block
scope use that same algorithm?
PHP literally frees all of the variables (even the ones which are definitely not reference counted)
without stopping if any of them throw an exception until everything is freed.
i_free_compiled_variables
is called when exiting the function scope (e.g. throw/return).
(the caller also decreases reference counts to the closure/$this, extra unexpected arguments, etc)
static zend_always_inline void i_free_compiled_variables(zend_execute_data *execute_data) /* {{{ */
{
zval *cv = EX_VAR_NUM(0);
int count = EX(func)->op_array.last_var;
while (EXPECTED(count != 0)) {
i_zval_ptr_dtor(cv);
cv++;
count--;
}
}
Having the variable become inaccessible but not actually deallocated
seems like it would cause a lot of confusion.
Good point about resources, I forgot about freeing being automatic.
I'm starting to lean towards deallocating.
If this was used within a try block or a top-level statement,
this could probably add a new opcode to free a variable that could throw
without checking for the exception,
and only the last opcode would need to throw the last exception.
(adding a new opcode to ensure deallocation of all variables without the equivalent of
a try/finally
block for every additional let
variable)
For global variables, there's already the problem in the engine that __destruct()
could itself create global variables of the same/different name.
- Tyson
Hi Tyson,
Normally I would be one to advocate for new features, but in the case I am against the proposed feature.
I know it is a feature from Javascript, but I think it is maybe the nature of Javascript prototypes makes it more useful though I am not sure as I do not specialize in JS. OTOH the nature of PHP coding minimizes the need.
More specifically regarding PHP, the purpose of your proposed feature appears to be to allow longer and more complex functions and methods and more deep nesting. However, I think we should not be encouraging people to write longer and more functions and methods and nest more deeply. And I have unfortunately have had to refactor too many long and complex functions and methods and deep nesting written by others that I actively want try to discourage such approaches to coding, in PHP at least.
What I would rather see are new features in PHP that encourage developer to break functionality into shorter and simpler functions and methods, and to minimize nesting.
The Software Engineering StackExchange seems to share my view:
https://softwareengineering.stackexchange.com/a/200712/9114
There is also an issue with block scoping that is reasonably well-known among the GoLang community called variable shadowning and that is a developer seeming a variable in block scoping and thinking they are accessing or assigning the function-scoped variable.
In GoLang it is more of a problem because of lack of explicit declaration required for variable shadowing — and the language designers have lamented that they wish they never included block scoping — but even though the original PHP developer might not get confused as they would be writing the `let's declaration another developer trying to maintain the code could easily introduce errors in the code if they don't notice the block declaration.
https://rpeshkov.net/blog/golang-variable-shadowing/ (https://rpeshkov.net/blog/golang-variable-shadowing/)
For these reasons I would definitely not want to see block-scoping of variables in PHP.
#jmtcw
#fwiw
-Mike
P.S. If you really need block scope within a long function or method you can already create a closure and call it in the same expression:
function Foo(){
$var = "I am a function scoped var!\n";
(function(){
$var = "I am a block-scoped var!\n";
echo $var;
})();
echo $var;
}
// Prints:
// I am a block-scoped var!
// I am a function scoped var!
Foo();
Hi internals, In PHP, variables are currently scoped to the function scope, and can't be scoped to a block scope. This makes it difficult to reason about how a variable will actually be used at a glance, especially in long functions, or top-level statement lists of a file. (or how the variable was originally intended to be used) The function scope lifetime of variables in PHP is similar to how JavaScript treated variables with
var
, before the introduction of thelet
statement in JS: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/let Would there be any interest in adding something similar to this in PHP? (Or have similar ideas been proposed in the past? I didn't see any.) Correctness would be enforced as a best-effort at compile-time - the variables would continue to only be freed at the end of the function call. (e.g. __destruct() only gets called when the variable gets reassigned or the function call ends) - Freeing it immediately would likely require the equivalent of a try{} finally{} to handle exceptions. try statements prevent opcache from optimizing a function, the last time I checked.{ let $myObject = new MyClass(), $flag = true; $myObject->process($flag); } // Accessing or setting $myObject or $flag outside a different let is an `E_COMPILE_ERROR` // because it was declared anywhere else in the function body // as a let statement $myObject->doSomething(); // `E_COMPILE_ERROR` to access $key or $value after, outside of a separate `let` scope foreach ($arr as let $key => let $value) { // It would be a compile error to declare a gotoLabel: inside of a scope of a let statement. } // `E_COMPILE_ERROR` to access $key or $value after echo $$var; // `E_COMPILE_ERROR` to use variable variables in the same scope, to enforce this as much as possible. { let $outer = 1; { let $outer = 2; // `E_COMPILE_ERROR` to declare in a different scope, can only reassign $outer = 3; // this is allowed } } { let $myRef =&$other; } { let $myRef = true; // This removes myRef from the reference group before assigning a new value to myRef - $other is unmodified. }
I say "best-effort" because require() and $GLOBALS and get_defined_variables() can still read or modify or make references to variables outside of alet
, depending on what's in the function body or top-level statements. - Tyson
Hi Mike,
What I would rather see are new features in PHP that encourage developer
to break functionality into shorter and simpler functions and methods, and to minimize nesting.
I would also like to see shorter functions in general, but:
- Other developers may prefer long functions/scripts despite my preferences,
but their code would still benefit from readability by having block-scoped variables enforced by the compiler - Some problem domains are complex (e.g. business logic) and require a lot of variables,
leading to long functions/scripts, and trying to split them up may be error-prone
or cause awkward divisions of functionality. - Even in short functions of a few dozen lines, it may be useful to know if reused variables such as
$i
are definitely scoped to a loop, when reviewing code.
There is also an issue with block scoping that is reasonably well-known
among the GoLang community called variable shadowing
and that is a developer seeing a variable in block scoping
and thinking they are accessing or assigning the function-scoped variable.
Variable shadowing is also an issue in JavaScript.
While JavaScript and GoLang don't allow introspection of the variable scope
(as far as I know, not sure about reflection),
PHP does allow dynamic access by name ($$var, get_defined_variables()['var']
in a require()d file, XDebug),
and any proposal for block scoping in PHP will need to account for that somehow.
- Aside: Personally, I set up linting for golang projects to forbid shadowing.
My best plan for dealing with variable shadowing in PHP was to forbid it entirely,
which I mentioned examples of in the proposal.
{
let $outer = 1;
{
let $outer = 2; //E_COMPILE_ERROR
to declare in a different (shadowed) scope, can only reassign
$outer = 3; // this is allowed
}
}
P.S. If you really need block scope within a long function or method you can already create a closure and call it in the same expression:
I'm aware of this and have used it (e.g. to avoid modifying the global scope in large projects),
but avoid it in functions due to the awkward syntax and the performance overhead of function calls.
I'd generally just use a different variable name.
- Tyson
Hi Mike,
What I would rather see are new features in PHP that encourage developer
to break functionality into shorter and simpler functions and methods, and to minimize nesting.I would also like to see shorter functions in general, but:
- Other developers may prefer long functions/scripts despite my preferences,
but their code would still benefit from readability by having block-scoped variables enforced by the compiler
What I am about to say is ironic since I too often feel people are trying to enforce their view of coding on the world, but I do not think we want to give people tools to make it easier to work with long functions/scripts.
And while I do understand that other people can disagree, please understand I am just stating my opinion here.
- Some problem domains are complex (e.g. business logic) and require a lot of variables,
leading to long functions/scripts, and trying to split them up may be error-prone
or cause awkward divisions of functionality.
I would be very interested in seeing business logic coded into a long function that literally cannot be made better by refactoring into multiple smaller functions.
If you have some, please do post to a Gist and share it to see if it in fact it cannot be refactored and made "better."
- Even in short functions of a few dozen lines, it may be useful to know if reused variables such as
$i
are definitely scoped to a loop, when reviewing code.There is also an issue with block scoping that is reasonably well-known
among the GoLang community called variable shadowing
and that is a developer seeing a variable in block scoping
and thinking they are accessing or assigning the function-scoped variable.Variable shadowing is also an issue in JavaScript.
While JavaScript and GoLang don't allow introspection of the variable scope
(as far as I know, not sure about reflection),
PHP does allow dynamic access by name ($$var,get_defined_variables()['var']
in a require()d file, XDebug),
and any proposal for block scoping in PHP will need to account for that somehow.
- Aside: Personally, I set up linting for golang projects to forbid shadowing.
My best plan for dealing with variable shadowing in PHP was to forbid it entirely,
which I mentioned examples of in the proposal.{
let $outer = 1;
{
let $outer = 2; //E_COMPILE_ERROR
to declare in a different (shadowed) scope, can only reassign
$outer = 3; // this is allowed
}
}P.S. If you really need block scope within a long function or method you can already create a closure and call it in the same expression:
I'm aware of this and have used it (e.g. to avoid modifying the global scope in large projects),
but avoid it in functions due to the awkward syntax and the performance overhead of function calls.
Given the nature of long functions, I think the awkward syntax here provides a nice disincentive to writing overly long functions.
But as I said, the closure does exist in case someone absolutely needs to control the scope of a block within an existing long function, such as the 1400 line function I've been refactoring for months now...
I'd generally just use a different variable name.
- Tyson
Hi Mike,
What I am about to say is ironic since I too often feel people are trying to enforce their view of coding on the world,
but I do not think we want to give people tools to make it easier to work with long functions/scripts.
Blocked-scoped variables is a tool which can make short code and long code more readable.
Making it easier to work with long functions is an example of a benefit of having block-scoped variables,
not the main goal of this RFC idea.
https://github.com/getify/You-Dont-Know-JS/blob/1st-ed/scope%20%26%20closures/ch3.md#blocks-as-scopes
goes into some of the other reasons to use block-scoped variables in JS
- Many languages other than JavaScript/PHP support Block Scope,
and so developers from those languages are accustomed to the mindset, whereas those who've primarily only worked in JavaScript may find the concept slightly foreign. - Block scope is a tool to extend the earlier "Principle of Least Privilege Exposure"
from hiding information in functions to hiding information in blocks of our code. - Developers may prefer to check themselves against accidentally (re)using variables outside of their intended purpose,
such as being issued an error about an unknown variable if you try to use it in the wrong place.
Block-scoping (if it were possible) for thei
variable would makei
available only for the for-loop,
causing an error ifi
is accessed elsewhere in the function.
This helps ensure variables are not re-used in confusing or hard-to-maintain ways.
I would be very interested in seeing business logic coded into a long function that literally cannot be made better by refactoring into multiple smaller functions.
If you have some, please do post to a Gist and share it to see if it in fact it cannot be refactored and made "better."
One example is if PHP itself is used for HTML rendering in top-level statements - pages or components of a page may be spread out across hundreds of lines.
This could be moved to a different templating engine for PHP instead of raw PHP,
but the refactoring would be time-consuming and error-prone.
Separately from HTML rendering, some problem domains are just complex.
For example, consider the functions such as decodeMPEGaudioHeader in
https://github.com/WordPress/WordPress/blob/master/wp-includes/ID3/module.audio.mp3.php
(I'm not involved in that project or file)
You could split it up into multiple helper functions, but instead of hundreds of lines in one function for a complicated file format,
you'd have to jump across even more abstractions/lines to know where XYZ was set.
Fully refactoring that might not make ever make business sense,
but there is a benefit to having the ability to mark if a variable is actually block-scope or used elsewhere
(e.g. to have a better idea of if refactoring to a different function is safe)
Given the nature of long functions, I think the awkward syntax here provides a nice disincentive to writing overly long functions.
But as I said, the closure does exist in case someone absolutely needs to control
the scope of a block within an existing long function, such as the 1400 line function I've been refactoring for months now...
Also, closures aren't a convenient substitute for everything the local scope can do for other reasons.
E.g. modifying by reference will mask issues with undefined or possibly undefined variables when modifying values outside of the closure scope.
function compute_product($values) {
// Other lines omitted
foreach ($values as $value) {
(function() use (&$product, $value) {
$letVar = $value + 1;
// Other lines omitted
$product *= $letVar;
})();
}
return $product;
}
This has the bug that $product was never defined, but no notices are emitted because $product gets set to null
when the reference is taken by reference, and php currently doesn't warn about multiplying by null.
Using an actual let $var = $value + 1
, this would get the notice for the variable being undefined.
- Tyson
Hi Mike,
What I am about to say is ironic since I too often feel people are trying to enforce their view of coding on the world,
but I do not think we want to give people tools to make it easier to work with long functions/scripts.Blocked-scoped variables is a tool which can make short code and long code more readable.
Making it easier to work with long functions is an example of a benefit of having block-scoped variables,
not the main goal of this RFC idea.https://github.com/getify/You-Dont-Know-JS/blob/1st-ed/scope%20%26%20closures/ch3.md#blocks-as-scopes
goes into some of the other reasons to use block-scoped variables in JS
- Many languages other than JavaScript/PHP support Block Scope,
and so developers from those languages are accustomed to the mindset, whereas those who've primarily only worked in JavaScript may find the concept slightly foreign.- Block scope is a tool to extend the earlier "Principle of Least Privilege Exposure"
from hiding information in functions to hiding information in blocks of our code.- Developers may prefer to check themselves against accidentally (re)using variables outside of their intended purpose,
such as being issued an error about an unknown variable if you try to use it in the wrong place.
Block-scoping (if it were possible) for thei
variable would makei
available only for the for-loop,
causing an error ifi
is accessed elsewhere in the function.
This helps ensure variables are not re-used in confusing or hard-to-maintain ways.
I hear you. Personally, I do not find any of those compelling enough to outweigh the concerns I have already raised.
But again, it is just my opinion and I respect your right and anyone else's to evaluate the pros and cons differently than I do.
I would be very interested in seeing business logic coded into a long function that literally cannot be made better by refactoring into multiple smaller functions.
If you have some, please do post to a Gist and share it to see if it in fact it cannot be refactored and made "better."One example is if PHP itself is used for HTML rendering in top-level statements - pages or components of a page may be spread out across hundreds of lines.
This could be moved to a different templating engine for PHP instead of raw PHP,
but the refactoring would be time-consuming and error-prone.
Since you did not provide any specific examples I will overlook this...
Separately from HTML rendering, some problem domains are just complex.
For example, consider the functions such as decodeMPEGaudioHeader in
https://github.com/WordPress/WordPress/blob/master/wp-includes/ID3/module.audio.mp3.php
(I'm not involved in that project or file)
Ironically, I work primarily with WordPress, and while I do so I find much of the code in WordPress core to be bordering on completely awful, and this code is no exception.
Looking at the decodeMPEGaudioHeader() function it is over 600 lines of meandering spaghetti, and a wonderful example of why enabling people to make these kind of functions more maintainable is akin to giving addicts the strongest, most addictive drugs imaginable. :-)
It would take more than 30 minutes to do a proper refactor, but here is just a sample of what could be done in half an hour; from 600+ lines to ~200:
https://gist.github.com/mikeschinkel/8bb52534836e3cd19eee7d4f3f5fa9fd
You could split it up into multiple helper functions, but instead of hundreds of lines in one function for a complicated file format,
you'd have to jump across even more abstractions/lines to know where XYZ was set.
I would argue that it is much harder to grok the entirety of the audio file by having to read a 600+ line function than if the main function called different functions that each handled a different part of the file.
In refactoring the above, I found clear and distinct areas to separate for LAME encoding and different variable bit rate encoding types. Also, this function is begging to be refactoring to use a variety of object classes, even if the output is covered to arrays for backward compatibility.
Instead of making it harder to understand the file format, breaking it out to multiple functions that mirror that different aspects of process a file will allow the developer to see the big picture — which is currently impossible with a 600 line function — and then to zoom into specific program functions when they want to understand more detail about a specific area.
Fully refactoring that might not make ever make business sense,
but there is a benefit to having the ability to mark if a variable is actually block-scope or used elsewhere
(e.g. to have a better idea of if refactoring to a different function is safe)
I totally get and agree with the concern about refactoring not making business sense. If you don't need to touch the code, don't touch the code.
But if you are going to modify the file to add block level declarations then you can just as easily refactor the area you would modify into its own function. Leaving as one large function means that you will likely never fully understand the patterns at play, and thus will never be able to simplify maintenance. And will almost certainly have to play whack-a-mole with bugs each time someone modifies it.
One thing is certain; you cannot reliably unit-test a long function like that; there is just too much going on to test in that function.
This type of long function is what has plagued my current client and has caused them to spend about 80% of their developer time fixing bugs rather than devoting most of their time to build new features to help them grow their business (they brought me on specifically to fix that problem, so seeing you trying to empower developers to create more spaghetti like that made me feel the need to speak out. But again, it's just my opinion.)
Given the nature of long functions, I think the awkward syntax here provides a nice disincentive to writing overly long functions.
But as I said, the closure does exist in case someone absolutely needs to control
the scope of a block within an existing long function, such as the 1400 line function I've been refactoring for months now...Also, closures aren't a convenient substitute for everything the local scope can do for other reasons.
E.g. modifying by reference will mask issues with undefined or possibly undefined variables when modifying values outside of the closure scope.function compute_product($values) { // Other lines omitted foreach ($values as $value) { (function() use (&$product, $value) { $letVar = $value + 1; // Other lines omitted $product *= $letVar; })(); } return $product; }
This has the bug that $product was never defined, but no notices are emitted because $product gets set to null
when the reference is taken by reference, and php currently doesn't warn about multiplying by null.Using an actual
let $var = $value + 1
, this would get the notice for the variable being undefined.
This example is too contrived for me to determine if there is an easy alternative coded it in a clearer manner that exist that does not have the concerns you mention. At first blush I would have the closure return a value and then do the multiplication outside the closure which solves your stated issue, but then the closure really does nothing, or at least nothing without the omitted lines included.
Still, I don't argue that you should use closures instead of refactoring to additional functions, just that they are there to use in case you are determined not to refactor into multiple functions.
#fwiw
-Mike