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

Partial application should be bound (like bound functions) #634

Closed
isiahmeadows opened this Issue Dec 24, 2014 · 4 comments

Comments

Projects
None yet
4 participants
@isiahmeadows
Contributor

isiahmeadows commented Dec 24, 2014

It's really confusing to have code like this constantly throw TypeErrors:

class Walker
    (@visitors) ->

    walkList: (list) !->
        list
        |> filter (.type of @visitors)
        |> each @walk

    # more methods

The problem is this partial function:

(.type of @visitors)

It looks like it should be bound to the current instance, equivalent to this code:

(node) ~> node.type of @visitors  # bound

It's really equivalent to this, which was a rather unexpected gotcha:

(node) -> node.type of @visitors  # not bound

This gotcha is a little misleading. It feels like a small fix, but I don't know a lot about the internal compiler structure.

@igl

This comment has been minimized.

Contributor

igl commented Dec 25, 2014

Aye. Those tiny implicit funcs should behave like es6-equal-rockets =>
Another fix would probably be not using classes and shitthis at all.

@vendethiel

This comment has been minimized.

Contributor

vendethiel commented Dec 25, 2014

Alright, I think I agree here.

@isiahmeadows

This comment has been minimized.

Contributor

isiahmeadows commented Dec 25, 2014

My initial workaround was this:

walkList: (list) !->
    {visitors} = @
    list
    |> filter (.type of visitors)
    |> each @walk

I ultimately ended up working around it by using inner functions, which helped me refactor it to be more functional rather than CoffeeScript-ish OO.

module.exports = (visitors, ast) !-->
   walkList = each walk, filter (.type of visitors)

   # more methods

The semantics seem to be a bug, potentially a design oversight in the language, but the workaround eventually helped surface a bad smell in my code.

I still say partial functions should be lexically bound (like ES6 arrow functions).

@summivox

This comment has been minimized.

Contributor

summivox commented Jan 7, 2016

Okay. After writing a patch that implements this behavior, I realized that it might not always be what you expect. Consider this:

o = a: 1, f: (@a =)

Here unbound this is intended.

My feelings:

  • A bound this feels "right" and might be more common [citation needed], since these implicit functions are intended for writing quick inline lambdas (where you would assume bound this), not methods
  • Syntax to support both bound and unbound might be overkill? Crazy idea: pick another sigil for bound this

@vendethiel vendethiel closed this in 9f96f95 Feb 1, 2016

vendethiel added a commit that referenced this issue Feb 1, 2016

Merge pull request #806 from summivox/bound-optr-fn
fixes #634 : Partial application should be bound (like bound functions)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment