Skip to content
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

Wrapping functions #118

Closed
zmthy opened this issue Jan 21, 2010 · 19 comments
Closed

Wrapping functions #118

zmthy opened this issue Jan 21, 2010 · 19 comments

Comments

@zmthy
Copy link
Collaborator

zmthy commented Jan 21, 2010

Is there a nice way to wrap large functions? I have a habit of going overboard with closures and it would be good to be able to wrap them easily, rather than
(=>
multiple_lines_of_code_here
)()
or when supplied as an argument
intFnString(5, (=>
multiple_lines_of_code_here
), "5")
Obviously this isn't a problem with single line functions.
intFnString(5, (=> 5), "5")

@weepy
Copy link

weepy commented Jan 21, 2010

I don't think there's a nice way. Main blocker really is a decent syntax ?!

@zmthy
Copy link
Collaborator Author

zmthy commented Jan 21, 2010

I wouldn't mind wrapping just the top layer.
(=>)()
multiple_lines_of_code_here
It's not overly confusing, just stating, "call this function I'm about to define".
When supplying an argument you could even drop the comma:
intFnString(5, (arg =>)
multiple_lines_of_code_here
"5")
There's no syntactical ambiguity as far as I can tell - correct me if I'm wrong, or just tell me it's a bad idea if it is.

@weepy
Copy link

weepy commented Jan 21, 2010

Thing is (=>) is already valid syntax, so I think your proposition would complicate the parsing quite a bit.

@jashkenas
Copy link
Owner

I'm a little confused -- Tesco, why would you need to make a closure wrapper in CoffeeScript? Let's see a real example. Passing a function to another is one thing -- and that's what the block syntax is for, but this is a bit stranger...

@zmthy
Copy link
Collaborator Author

zmthy commented Jan 21, 2010

for method in methods
  name: method.name
  Klass.prototype[name]: =>
    some_code()
    something[name].call(this)

Without closure, name continues to change and the inner code will always call the last name that exists on something[name]. If we wrap it, however
for method in methods
(=>
name: method.name
Klass.prototype[name]: =>
some_code()
something[name].call(this)
)()
each name is an individual variable and retains its value for each iteration.
Also - I like hiding things from myself. I'm weird like that.
Yes, weepy, (=>) is already defined, but there's no way an indent can exist directly after it (as far as I can think of). That said, I have no idea how the parsing works, so it's entirely possible that it's a ridiculous proposal.

@jashkenas
Copy link
Owner

It's by no means a ridiculous proposal -- but part of the idea of CoffeeScript is that you shouldn't have to closure-wrap yourself in most cases, because we're going it for you when required. So let's look at an alternative:

for method in methods
  Klass.prototype[method.name]: =>
    some_code()
    something[method.name].call(this)

If you're using method.name multiple times within the function body -- you can always set name inside of there, without having to worry about sharing variables. Does that work alright for you? I agree that each semantics are nicer than for loops, so perhaps if you're targeting the server-side with your class, you'd like to:

methods.forEach() meth =>
  name: method.name
  Klass.prototype[name]: =>
    some_code()
    something[name].call(this)

It's a good bit longer that way, though.

@zmthy
Copy link
Collaborator Author

zmthy commented Jan 21, 2010

Doesn't that first example have precisely the same problem? method is defined at the top of the scope - outside of the loop - and hence something[method.name] is still the incorrect name.

@jashkenas
Copy link
Owner

Oy. It sure does have the same problem. Sorry about that. Your original wrapping proposal is the only way to make this work. One issue with defining a new syntax is that, once you need to add the wrapper, you might as well just use the equivalent each, which will both do what you want and run in native code.

@zmthy
Copy link
Collaborator Author

zmthy commented Jan 21, 2010

each isn't always equivalent though. We obviously go about things a little differently - I like using closures just to hide variables. In the same way that it's a bad thing to clutter the global namespace, and why CS has the default wrapper, I like to keep any scope I'm working in clean.
params: {}
params_array: getParams()
i: -1; while ++i < params_array.getLength()
name: params_array.item(i)
params[name]: getParamValue(name)
This is a bit of an obtuse example, but my point is that I've just dumped messy variables all over my scope. So I write this:
params: (=>
ret: {}
...
ret
)()
This is actually one of the things I like least about CS - that I can't write
value: (=> value: {})()
and have the inner value be a different variable than the outer. That's beside the point, however, as I can cope by just renaming things. The point is that I use closures in a way that CS doesn't provide a way around, and I find the syntax for it a little unpleasant. It's a trivial matter, really, and I love CS as a whole, but I thought I'd bring it up and see if there was a chance that it could be addressed.

@jashkenas
Copy link
Owner

I don't think it's trivial at all. Since the current syntax for this is clunky (albeit still streamlined when compared to JavaScript), I'd love to see what your ideal syntax for this purpose would look like. It seems like what you need is:

  • A way to mark off a block of code as a closure for immediate evaluation.
  • A way to specify the return value of the block.

At first blush, it seems like we could easily use the block-indented scope to make a really nice construct for this.

@weepy
Copy link

weepy commented Jan 21, 2010

If the proposed (=>)() was used, it could be generallized for more args:

fn2: (=>)(x,y)
          ...

@weepy
Copy link

weepy commented Jan 21, 2010

or perhaps just

fn: =>()
        ...
fn2: =>(x,y)
        ...

So if you wanted to port jQuery (I know it does quite work...), you might do:

window, undefined =>(window)
  ...

@hen-x
Copy link

hen-x commented Jan 22, 2010

In the specific case of capturing loop variables in an inner function, how about automatically providing a closure wrapper? e.g.

for item in list
    doSomething(item)

for item in list
    doSomething() =>
        alert(item)

...

var __a, __b, __c, __d, item;
__a = list;
for (__b = 0; __b < __a.length; __b++) {
  item = __a[__b];
  doSomething(item);
}

__c = list;
for (__d = 0; __d < __c.length; __d++) {
  (function(){
    var item;
    item = __c[__d];
    doSomething(function() {
        alert(item)
    });
  })();
}

@jashkenas
Copy link
Owner

Fantastic, thanks sethaurus. No new syntax required, and no special thought. Now, on master, if you declare functions within the body of an array comprehension, an extra closure wrapper will be generated to prevent your variables from being shared with subsequent iterations. Here's the test case, which prints all true:

obj: {}

methods: ['one', 'two', 'three']

for method in methods
  name: method
  obj[name]: =>
    "I'm " + name

print(obj.one()   is "I'm one")
print(obj.two()   is "I'm two")
print(obj.three() is "I'm three")

Closing the ticket... Let me know if it doesn't work for you, Tesco.

@zmthy
Copy link
Collaborator Author

zmthy commented Jan 23, 2010

I'm a little concerned about it automatically generating closures based on the loop's contents - for example, what if I wanted the variables to not be enclosed in the closure? How might I go about negating the effects of the automatic closure? What you're achieving is different to how Javascript works, and having it change invisibly seems dangerous to me.
Also, if it's generated in the same way as above, you've just lost the context.
cxt = this;
(function () {
for (i = 0; i < list.length; ++i) {
innerCxt = this;
(function () {
var item = this;
fn = function () { return item; };
}());
}
}).call({});
fn() === cxt; // true
fn() === innerCxt; //false

@jashkenas
Copy link
Owner

Totally dangerous, but we're already doing it (injecting closures) all over the place, in order to transform statements into expressions, which includes correcting inner references to "this" when we need to. In this case, the "this" of the function is going to become the object that it's attached to, because of JS's dynamic scope for "this", regardless of what its value is at definition time.

The main issue here is sharing variables between iterations -- it should never be a problem for ordinary loops, because the variables are assigned and used immediately, and disappear at the end of the block. On the other hand, when you create a function within a loop, you probably expect the values of the variables created not to change out from right underneath your feet. For comparison, if you run the same pattern in Ruby, you get this:

obj = {}
methods = ['one', 'two', 'three']
methods.each {|m| obj[m] = lambda { puts "I'm #{m}" }}

obj['two'].call
=> "I'm two"

Sure, it isn't one-to-one JS semantics, but seems more inline with expectations of lexical scope. If its too dangerous, we can absolutely revert it. But we should talk about a real-world use case where it doesn't do what you want.

@zmthy
Copy link
Collaborator Author

zmthy commented Jan 24, 2010

Oh, I see, you manually correct this. Surely any automatic closure could just be written
(function () { this; }).call(this);
to preserve the context, rather than
(function (__this) { __this; })(this);

@jashkenas
Copy link
Owner

Quite right. This is much better than rewriting this-references. I think originally I had a concern about preserving this through nested closures, but if we do this every time we generate one, it'll keep the chain intact. Many thanks, it's now on master.

So, apart from this, the only other side effect I can think of is the introduction of a new scope, which is something that we also have a way around, if we need it. (Sharing the parent Scope object, instead of creating a new one). However, the new scope is the desired effect in this case.

@zmthy
Copy link
Collaborator Author

zmthy commented Jan 24, 2010

Righto, I have no problem with this.
However, should we be taking into account outer access of the variable?
obj: { prop: "value" }
for i of obj
item: obj[i]
fn: => item
item # value
At the moment that would result in
for (i in obj) {
(function () {
var item = obj[i];
fn = function () { return item };
}).call(this);
}
item; // ReferenceError
If so, we should have two parts to the inner declarations:
for (i in obj) {
item = obj[i];
(function () {
var item = obj[i];
fn = function () { return item };
}).call(this);
}
item; // value
If fn isn't defined outside of the loop you could potentially lose that as well.
We should also trap the variables declared in the for as well.
for (i in obj) {
__a = i;
(function () {
var i = __a;

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants