-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add "procedures": functions that are guaranteed to not return a value #2726
Conversation
"Procedures" are functions that do not implicitly return a value: coffee> test = ->> 'hi' [Function] coffee> test() undefined They can, however, manually return a value: coffee> test = ->> return 'hi' [Function] coffee> test() 'hi' Procedures are created with the '->>' glyph. Like regular functions, they can be bound to the current "this" context, using the '=>>' glyph.
Another solution, which I'm now partial to, would be to outright ban RationaleCoder Context Switches and Asynchronous FunctionsAsynchronous functions that operate with the standard callback mechanic should not themselves return a value - the passing back of data should be handled exclusively through the specified callback. Yet, with the current state of the language, ensuring this requires a context switch / backtracking when writing a function. Consider: readFiles = (directory, cb) ->
fs.readdir directory, (err, files) ->
if err?
cb err
else
output = {}
for file in files then do (file) ->
fs.readFile path.join(directory, file), 'utf8', (err, data) ->
if err?
cb err
else
output[file] = data
files.pop()
cb undefined, output if files.length is 0
return The As it is, also, the callbacks to the readFiles = (directory, cb) ->
fs.readdir directory, (err, files) ->
if err?
cb err
return
else
output = {}
for file in files then do (file) ->
fs.readFile path.join(directory, file), 'utf8', (err, data) ->
if err?
cb err
return
else
output[file] = data
files.pop()
cb undefined, output if files.length is 0
return
return
return More backing up, more complexity, and more painful indentation tracking. All those readFiles = (directory, cb) ->>
fs.readdir directory, (err, files) ->>
if err?
cb err
else
output = {}
for file in files then do (file) ->>
fs.readFile path.join(directory, file), 'utf8', (err, data) ->>
if err?
cb err
else
output[file] = data
files.pop()
cb undefined, output if files.length is 0 Combining the callbacks and the return statements into one line reduces complexity even further. readFiles = (directory, cb) ->>
fs.readdir directory, (err, files) ->>
(cb err; return) if err?
output = {}
for file in files then do (file) ->>
fs.readFile path.join(directory, file), 'utf8', (err, data) ->>
(cb err; return) if err?
output[file] = data
files.pop()
cb undefined, output if files.length is 0 This way, ReadabilityCurrently, the same can be accomplished by adding a The addition of a separate symbol allows for the intention of a function to be enforced at its definition. Existence within the CoffeeScript Design PhilosophyAn oft-espoused principle of implicit returns is that they force one to consider good return values when writing a function. I believe that the addition of first-class procedures is complementary to, rather than contradictory of, this design tenet. The choice between the two types of function glyphs forces the consideration of the return type and purpose of a function from the very beginning. Rather than simply turning off implicit returns, these procedures are guaranteed not to return a value, in a way far more explicit than the addition of a Avoidance of Inadvertent Implicit ReturnsCompatibility with Testing FrameworksSeveral testing frameworks (and other libraries, for that matter) are sensitive to the return types of functions - see vows. vows = require 'vows'
assert = require 'assert'
vows.describe('Example Case').addBatch
'example topic':
topic: -> someAsyncFunction this.callback
'returns true': (topic) -> assert.equal topic, true In vows, "topics" for tests can be calculated asynchronously; this relies, however, on a function returning The addition of a separate symbol for procedures prevents such problems from occurring in the first place, as the decision of whether or not a function should return a value is handled explicitly at its definition. PerformanceIt's been said before: the way loop comprehensions are done means that the usage of loops in functions can result in the unintentional construction of arrays. unlinkFiles = (files) -> fs.unlinkSync file for file in files
# returns an array - most likely not what was intentioned
unlinkFiles = (files) ->
fs.unlinkSync file for file in files
return
# does not return an array
# intention to do so is specified indirectly, rather than directly
# cannot be a one-liner, unless semi-colons are used
unlinkFiles = (files) ->> fs.unlinkSync file for file in files
# clearly denoted as a procedural function
# remains a one-liner API DesignThe explicit demarcation of functions as procedural prevents the leakage of internal values, which could lead to unintended usage / breakage as internal implementations change and evolve.
|
Why give that option? If someone wants to return a value, he can just use a regular function. If I see |
@shesek See my second comment in the issue - this is exactly the direction I'm heading in with this. |
Ah, right - "Rather than simply turning off implicit returns, these procedures are guaranteed not to return a value". sorry, I missed that :) |
Yay! I must say that i'm not strongly in favour or against having a syntax for procedures. But after that awesome rationale of yours, i agree: it can make the language much better for some of people (maybe lots of people, how knows? =P). You've got my vote 👍 |
About the cb err and return if err? You can write: (cb err; return) if err? Maybe not as English-like readable as the first snippet; but it's an option to avoid some of the nesting. |
+1 for proposal: CS has needed something like this for a long time. Coco's |
Anyway, it's just a very minor peeve. |
Generalize the routine used to block constructors from returning values to forbid the same in all Code blocks marned as `@noReturn`. Throw a SyntaxError in this case.
I've changed it a bit so that procedures are forbidden from returning a value; doing so will now throw a @epidemian I agree that |
I'd vote for |
– We should use procedures here. xD But yeah, i played a little bit in the text editor and i'm quite convinced that We kinda need the And the
And having more than 2 different (non-alphanumeric) symbols for the same token would make it very inconvenient in my opinion. I think the use of procedures could turn out to be very common (e.g. in all kinds of unit testing code, callbacks, etc), so i'd prefer their symbol to be as keyboard friendly as possible. |
Coming to think about this a bit more...
Also, what about the more general case of using [1] 355 CoffeeScript repositories according to https://github.com/search?q=%22return+cb+err+if+err%22&ref=commandbar&type=Code |
Only returns with values are forbidden inside procedures at the moment, so value-less returns (for flow control) still work as usual. If #2729 doesn't make it in, you could still do, as you mentioned before: (cb err; return) if err? inside a procedure, and that would work. |
I'm afraid I'm going to have to close this for the same reasons as before -- It's a great PR, and a great example implementation, but the reasoning for the current function options haven't changed. Ideally every function returns a meaningful value -- if it's asynchronous, maybe a promise -- even if that value is just |
Aw, shucks. Was worth a go! |
I certainly appreciate the time and thought you put into this pull request, @mintplant. For what it's worth, I find your points compelling. :) |
"Procedures" are functions that do not
implicitlyreturn a value:They can, however, manually return a value.Trying to return a value from a procedure will throw a SyntaxError, as it does if you try to return a value from a constructor:Procedures are created with the
->>
glyph. Like regular functions, they can be bound to the current "this" context, using the=>>
glyph.I figured I'd get the ball rolling on this by putting together some code. It was a relatively simple change - just adding in the additional symbols, and having the
Code
node set the@noReturn
flag totrue
if the tag isprocedure
orboundprocedure
. Tests are included, and it passes all current ones.I know this has been brought up before (specifically in #899, #1812, #1836, #1855,...), but perhaps with a concrete implementation, this could actually go somewhere.
Thoughts?