gorillascript icon indicating copy to clipboard operation
gorillascript copied to clipboard

Hash-functions should be bound to 'this' by default

Open andreyvit opened this issue 12 years ago • 13 comments

I've been playing with GorillaScript, and I strongly believe that #-functions should always be bound to the outer this, otherwise it would be a constant source of errors just like => / -> in CoffeeScript.

Something like the following code feels very natural, and forgetting to put the @ sign in #(el)@ would obviously be one of the top errors, which does not fit into the concept of being safe by default:

class Foo
  def constructor(@threshold)
    pass  // an empty body crashes the web compiler for me

  def process(list)
    list.filter(#(el) el > @threshold)

The cases when you specifically need an unbound anonymous callback function are very rare and mostly specific to jQuery, while pretty much any callback you write inside a class wants to be bound. I'd argue that jQuery use case should require special syntax, while the common use case should use plain #.

Note that in the current shape of ES.next, they made exactly the same decision by accepting => and not ->.

Also writing -> instead of => is one of the top CoffeeScript mistakes for me (another one being forgetting an @ when calling a method from the same class). In CoffeeScript it's especially bad because you use -> to define class methods, so you can't just train yourself to always type =>. But even in GorillaScript, I can see myself saying dirty things every time I forget that extra @, which would be very often.

andreyvit avatar Jul 07 '13 19:07 andreyvit

Also, GS macros already behave this way, as evident from jsprelude.gs:

macro return?
  syntax node as Expression
    @mutate-last node,
      #(subnode)
        @maybe-cache subnode, #(set-subnode, subnode)
          AST
            if $set-subnode?
              return $subnode
      true

If this was non-macro code, the at-calls would fail.

So even the compiler code shows that bound callbacks are expected by default. :-)

andreyvit avatar Jul 07 '13 19:07 andreyvit

Well, I definitely wouldn't want it to act differently with a # function vs. let vs. def. def always should default to having its this be normal-bound.

I don't especially want to special-case this, in the instance of:

class Foo
  def method = promise! #(value)
    /* promise-related code in here, using `this` */

I'm not sure I'd rather be writing

class Foo
  def method()@
    @blah()

all the time.

ckknight avatar Jul 07 '13 23:07 ckknight

@ckknight Right, method lambdas are tricky. But still, if you forget about the edge cases, don't you agree that a bound callback is the most common use case and should be a simple language construct?

andreyvit avatar Jul 08 '13 00:07 andreyvit

I am contemplated to lean that way, but I could only see myself doing that if I were to reverse the role of @ on a function declaration, so that

let normal-this()@
  this

let parent-this()
  this

let obj = {}
obj == normal-this@ obj
window == parent-this@ obj

i.e. the exact opposite of what is there now.

I wouldn't want to special-case def, as I feel it would lead to more confusion than not.

ckknight avatar Jul 08 '13 03:07 ckknight

I would like to get some community feedback on this before I make a decision either way.

Comment with "+1" to change functions to require the @ for normal JavaScript this-binding, without for the this being that of the outer scope. Comment with "-1" to keep functions as they are, with @ making this refer to that of the outer scope and nothing for normal JavaScript this-binding.

ckknight avatar Jul 13 '13 03:07 ckknight

+1

dottedmag avatar Jul 13 '13 08:07 dottedmag

-1 at first thought.

In CoffeeScript I use the => (fat arrow) very sparingly, and I prefer not to have my every simple lambda with a _bind overhead. It seems to me this binding behavior is much more prevalent in object-oriented code, which I use little in favor of functional code, and I like how GorillaScript allows for the latter easily.

I think reversing this would create a lot of confusion. Would the reversal be just for anonymous functions? That would be the very confusing. If it's for every function, well, I frankly use the actual current 'this' a lot, lot more than a parent's this.

Why not a solution like CoffeeScript's, so that to create a bound anonymous function you'd use another syntax (like the fat arrow)? We could have (# => "hello world") and #(world) => "hello $world". Plus it's an idiom people likely to use GorillaScript are familiar with (assuming many come from CoffeeScript.) It keeps simple, quick and dirty anonymous functions as lightweight as possible--they're definitely what I use the most in terms of creating functions.

As a question, now, I'm seeing precisely issues with 'this' being undefined in the main scope of my function created with 'let my-func()'. When I declare it as 'let my-func()@' I get the window object as should be. Am I not seeing something obvious?

goldfeld avatar Jul 13 '13 19:07 goldfeld

@goldfeld: There is no bind overhead, it simply uses var self = this in the outer scope and then replaces this with self in the bound inner scopes at codegen time. So it will not affect your lambdas, unless you specifically need an unbound this pointer.

Does that alter your opinion?

andreyvit avatar Jul 14 '13 11:07 andreyvit

Also, if this is never used in the function, there will be absolutely no overhead, not even the need for

var _this = this

In the outer scope.

ckknight avatar Jul 14 '13 13:07 ckknight

Ok, so that's not a valid objection to the proposed change. Still, I'm thinking that this will start causing usage errors for people like me and anyone already used to the standard behavior in, well, all of JavaScript and mostly all of it's derived languages. As @ckknight has said, and I agree, this change would better contemplate all function declarations, not just anonymous ones.

What it means, as he exemplified above, is that by default 'this' now would always refer to the global/window object (I'm not considering classes as I haven't seen how they work w.r.t. that in GorillaScript yet, and I'm more worried about plain old functions.) What's the use in that? If I want to refer to the window object in a client-side situation, I just use 'window'. Why have another keyword to mean precisely the same?

I'm not a big fan of the let my-func()@ syntax to be honest, and I would really dislike defaulting to using it all the time (which I would do) just so that I would not now have to be having the same class of errors that @andreyvit outlined in the first post. I don't know, regardless of overhead, it just feels wrong to have to add syntax so as to do less. More importantly, to add syntax in order to go back to JS and CS (and so far GS)'s default behavior. People (and beginners who suffer the most from this) should adapt to the JavaScript environment and how scope is handled, not the other way around, at least in my opnion--every language has it's particularities and CoffeeScripters (me included) seem fine with the fat arrow solution. I don't think it should be inverted.

In light of the problem I cited above, where 'this' would always equal to 'window' unless unbound, I think this really is a solution contemplating only object-oriented code (as how is having 'this == window' useful in ordinary code?), and I don't think GorillaScript should optimize for object oriented code, at all. If anything, I'd love for it to optimize for really functional code (as it has to many degrees, with easy currying and composing, as well as higher-order-like behavior emulated into for loop syntax), because that's an unfilled niche in the JS ecosystem (CoffeeScript has terse function syntax, but otherwise "it's just JavaScript".)

So considering now that this change is only useful inside a class, why not explore alternative solutions specific to classes? For instance, maybe we could have a built-in called 'self' that would always refer to the current class' instance. I'd also have no qualms with the '@' symbol meaning self inside classes. I think that's how they call it in python and I have done my fair share of using the 'self' idiom in KnockoutJS view-model code, so it's relatively known to people who work with OO. You would use 'self' and never have problems. But please leave this out of it (pun intended.)

I'm really open to further discussion and please point anything I might have gotten wrong.

goldfeld avatar Jul 15 '13 06:07 goldfeld

@goldfeld Good point about optimizing for a functional style. But. Would this change really have any effect on that? I think right now we should pull jashkenas on us and ask for real-world code examples to support the arguments.

Mind you, I have a nice aging code base written in mostly functional style. I've been slowly transitioning it to an OO style because the functional one was becoming a maintenance problem, but I only have 15 classes in 2 hierarchies so far (Layout and Snappings). In that code base:

  • There are 6500 lines of CoffeeScript with 818 functions.
  • Of those 818, 19 functions would be affected by this change:
    • 12 are custom jQuery plugins (jQuery.fn.smt = ->) and require unbound this; these are top-level functions and would only be affected if we also change the behavior of let;
    • 5 are calls to jQuery.each that would need to accept real arguments or use @;
    • 2 are jQuery event handler calls that require unbound this and would have to use @.
  • Also, of those 818, 5 functions currently use CoffeeScript's fat arrow (=>).

So: for a mostly purely functional code base, if we only change #-functions, 7 of 818 (0.8%) are negatively affected, and 5 of 818 functions are positively affected. If we change let along with #, 2.3% functions would be affected negatively. In every case, the problem comes from using jQuery and is trivial to spot.

Anyone wants to do the numbers for their code? I'll try to provide the numbers for OO code style later.

andreyvit avatar Jul 15 '13 10:07 andreyvit

...In fact, when I accept this from jQuery, I always have a desire to somehow highlight that fact. I kinda like ES.next discussions around adding this to the arg list for that. So instead of:

$('input').each #@ @hide()

you do something like

$('input').each #(@) @hide()

or

$('input').each #(this) @hide()

Note that this has not been approved for ES.next; they felt that this argument is too confusing. But again, for those very rare use cases where some magic this comes from the library, it is really an argument passed in a funny way.

andreyvit avatar Jul 15 '13 11:07 andreyvit

As I said before, if I make this change, it will affect all function declarations, not just anonymous (#) functions or def or let separately.

ckknight avatar Jul 15 '13 17:07 ckknight