Javascript and Callbacks

Question

I wrote a JavaScript function, and it was working perfectly, but it was pretty long, so I wanted to break it up into smaller functions. I thought this would be easy (and maybe it is) but I'm running into issues!

So the structure of my code is as follows:

getPosition: function(a) {
    if (true) {
        position = this.getPoint(a); 
    }
}, 
getPoint: function(a) {
    var position; 
    var options = a.target.parentElement.children; 
    [].forEach.call(options, function(option){
        if (option.type == "point") {
            position = this.getNewPoint(a, option); 
        } else if (option.type == "line") { 
            position = this.getNewLine(a, option); 
        }
    }
    return position; 
}, 
getNewPoint: function(a, option){
    ...
    return point; 
}, 
getNewLine: function(a, option){
    ...
    return line; 
}

Trying this gave me the error that this.getNewPoint and this.getNewLine were not defined. That makes sense because of scope, so I decided to try using a callback:

getPosition: function(a) {
    if (true) {
        position = this.getPoint(a, this.getNewPoint, this.getNewLine); 
    }
}, 
getPoint: function(a, pointCallback, lineCallback) {
    var position; 
    var options = a.target.parentElement.children; 
    [].forEach.call(options, function(option){
        if (option.type == "point") {
            position = pointCallback(a, option); 
        } else if (option.type == "line") { 
            position = lineCallback(a, option); 
        }
    }
    return position;
}, 
getNewPoint: function(a, option){
    ...
    return point; 
}, 
getNewLine: function(a, option){
    ...
    return line; 
}

This get's the functions to be called as wanted, however, (I'm guessing that) because of the asynchronous nature of Javascript, the code is continuing without the callbacks completing, so the return value is never being returned. It looks like when I put in some console.log() to test it it started working. I'm guessing this is because it's forcing the code to slow down. Any ideas?

UPDATE: Because of the awesome help I've received I've got it working to a point. So right now it works as long as the getNewLine function is never called. So I'm thinking there's something wrong in the way my getNewLine function is returning, since it's breaking everything! So here's a but more detail on that function:

getNewLine: function(a, option){
    var line; 
    var endPoints = option.points; 
    for (var i = 0; i < (endPoints.length - 1); i++) { 
        ... math here 
        if (distance <= linePadding) { 
            if (true) {
                line = option; 
                return line; //Want to break the loop and for the function to return 
            }
        } 
    } 
    return line; 
}

You should have a look at [How to access the correct `this` / context inside a callback?](https://stackoverflow.com/questions/20279484/how-to-access-the-correct-this-context-inside-a-callback) — Felix Kling, Feb 03 '16 at 16:14
`getPoint()` doesn't return anything! What exactly are you trying to do here? — gen_Eric, Feb 03 '16 at 16:15
@RocketHazmat Sorry about that, in my actual code it does return. I've updated it — Sara Fuerst, Feb 03 '16 at 16:17
What *exactly* do you want `getPoint()` to return? Why are you using `forEach`, but only returning one value? Maybe try a *normal* `for` and `break;` once you've found the `position`? — gen_Eric, Feb 03 '16 at 16:44
@RocketHazmat I want `getPoint()` to return `position`, which will be either a point or a line — Sara Fuerst, Feb 03 '16 at 16:46
@tibsar: Then you probably want to use `for(...)` instead of `forEach`. — gen_Eric, Feb 03 '16 at 16:47
@RocketHazmat I see what you're saying. Basically Once `position` is defined, I want it to be returned. Because of my forEach loop, it will not necessarily return what I think it's returning. So once `position` is defined, I need to break out of the loop. What's the cleanest way to do that? — Sara Fuerst, Feb 03 '16 at 16:50
@tibsar: That *may* be your issue, yes. `forEach` cannot stop prematurely. So, you may be constantly overwriting `position`. (P.S. Go Tigers! I'm also an RIT student (well, alumni)! :D) — gen_Eric, Feb 03 '16 at 16:52
@RocketHazmat small world! thanks for the help! I bet that is the issue! — Sara Fuerst, Feb 03 '16 at 16:53
@RocketHazmat random question: Were you a part of MDRC? You look crazy familiar — Sara Fuerst, Feb 03 '16 at 17:54
@tibsar: Yes I was! :-D (I'm Eric, pretty sure I know you...) — gen_Eric, Feb 03 '16 at 18:16

score 2 · Accepted Answer · edited May 23 '17 at 12:15

2

You haven't introduced anything truly asynchronous, just a different method of applying a function. There's a much easier fix to your problem than using callback; save a reference to the correct this:

getPoint: function(a) {
    var self = this; // <--
    var options = a.target.parentElement.children; 
    [].forEach.call(options, function(option){
        if (option.type == "point") {
            newPoint = self.getNewPoint(a, option); // <--
        } else if (option.type == "line") { 
            newLine = self.getNewLine(a, option); // <--
        }
    });
},

As for why your new solution doesn't work, it looks like you aren't passing the callbacks with the right context. First off, I believe you meant to type

position = this.getPoint(a, this.getNewPoint, this.getNewLine);

But the problem with this is that you, again, lose the correct this context. You could fix this by explicitly setting it using .bind

position = this.getPoint(a, this.getNewPoint.bind(this), this.getNewLine.bind(this));

Bind creates a copy of the given function where the this context is explicitly set.

I actually wrote an answer explaining how this is determined here. And as Felix Kling pointed out, .forEach accepts another argument which sets the context of this:

[].forEach.call(options, function(option) {
  // Your same code as before
}, this); // <-- Set the context

edited May 23 '17 at 12:15

Community

1
1

answered Feb 03 '16 at 16:10

Mike Cluck

31,869
13
80
91

2

Or use an arrow function if you are working in an environment that lets you! – simon-p-r Feb 03 '16 at 16:13
In this case it would be more elegant to pass `this` as second argument to `forEach`. – Felix Kling Feb 03 '16 at 16:16
1

That is a matter of opinion – simon-p-r Feb 03 '16 at 16:18
I just tried saving `this` as a variable, and passing it in (instead of using callbacks). It seems to be calling the functions, but I'm still having the issue where it seems to be returning too early. – Sara Fuerst Feb 03 '16 at 16:20
2

@tibsar It's not returning too early, you're declaring `position` inside of the `forEach` function. Declare it outside of the `forEach` and you'll be golden. – Mike Cluck Feb 03 '16 at 16:22
@MikeC That helped! So I tried that and tested it and it seems that works! It looks like I have a bigger issue though, because when just the `getNewPoint()` function is called it works, the `getNewLine()` function is called, it stops working for both the point and the line. – Sara Fuerst Feb 03 '16 at 16:28

gen_Eric · Answer 2 · 2016-02-03T16:36:28.160

1

There is no asynchronous code here. Just because you are passing a function as a parameter doesn't mean it's asynchronous.

~~The issue you are having is that getPoint isn't returning anything! You need a return statement for it to return anything.~~

As for the first example, the value of this changes every time you enter a new function(){}. this is based on how the function is called. Inside the forEach, this is ~~the element in the "array"~~ the global window object, not your object.

~~You can "backup" this to a variable and then use that inside the forEach.~~ You can set the value of this in the forEach by passing it after the callback.

getPoint: function(a) {
    var options = a.target.parentElement.children,
        position;

    [].forEach.call(options, function(option){
        if (option.type == "point") {
            position = this.getNewPoint(a, option); 
        } else if (option.type == "line") { 
            position = this.getNewLine(a, option); 
        }
    }, this);

    return position;
},

edited Feb 03 '16 at 16:36

answered Feb 03 '16 at 16:14

gen_Eric

223,194
41
299
337

In this case it would be more elegant to pass `this` as second argument to `forEach`. – Felix Kling Feb 03 '16 at 16:16
@FelixKling: Didn't realize `forEach` had a context parameter! – gen_Eric Feb 03 '16 at 16:16
In my actual code I am returning. Sorry about that. I updated the post. – Sara Fuerst Feb 03 '16 at 16:20
1

@tibsar: Either way, you just need to pass `this` to `forEach` after the function and it should work. – gen_Eric Feb 03 '16 at 16:39
@RocketHazmat I ended up passing in this and that did seem to work, however, I think my return issues may go deeper since now it works as long as I don't call `getNewLine()`. I've updated the question to show a bit more detail about how my function is operating – Sara Fuerst Feb 03 '16 at 16:41

Dustin Poissant · Answer 3 · 2016-02-03T16:26:15.503

-3

What you think this is is changing scopes. this always points to the most recent parent function. So if you separate the code out into multiple functions the this variable changes depending on the function.

One way to work around this is, when calling one function from another you can set the this within the function using the .call or .apply methods.

.call takes the this scope as the first parameter, and all following parameters are the actual parameters passed into the called function. (argument = parameter -1).

.apply takes the this scope as the first parameter, and its second parameter is an array that will be passed into the called function as it's parameter.

So in this case I would suggest

getPosition: function(a) {
    if (true) {
        position = this.getPoint.call(this, a); 
    }
}, 
getPoint: function(a) {
    var position; 
    var options = a.target.parentElement.children; 
    [].forEach.call(options, function(option){
        if (option.type == "point") {
            position = this.getNewPoint.call(this, a, option); 
        } else if (option.type == "line") { 
            position = this.getNewLine.call(this, a, option); 
        }
    }
    return position; 
}, 
getNewPoint: function(a, option){
    ...
    return point; 
}, 
getNewLine: function(a, option){
    ...
    return line; 
}

edited Feb 03 '16 at 16:26

answered Feb 03 '16 at 16:14

Dustin Poissant

3,201
1
20
32

2

*"`this` always points to the most recent parent function."* That's confusing. `this` rarely refers to a function. – Felix Kling Feb 03 '16 at 16:15
`this` always refers to an prototype instance, and functions are instances of the `Function` prototype. So `this` often referes to a function – Dustin Poissant Feb 03 '16 at 16:16
1

Oh really? Calling methods on objects is a very common operation: `var foo = {bar: function() {}}; foo.bar();`. What do you think `this` refers to inside `bar` here? – Felix Kling Feb 03 '16 at 16:17
If written inside the `bar` definition then yes it does – Dustin Poissant Feb 03 '16 at 16:17
2

*"If written inside the `bar` definition then yes it does"* That's not an answer to my question. Either way, in my example `this` will refer to `foo`, which is an object, which is not a function. I highly recommend to read the [MDN documentation](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/this) if you are not very familiar with `this`. – Felix Kling Feb 03 '16 at 16:18
Except that when you enter a function that is a member of an object, and reference this inside that function, then `this` is changed to point to that function member and not the parent object. – Dustin Poissant Feb 03 '16 at 16:18
So you deny that in my example `this` would refer to `foo`? – Felix Kling Feb 03 '16 at 16:19
That depends where you call it, if you call it just inside foo and outside bar (as a sibling of bar) then it points to foo. But once you enter bar it now points to bar. – Dustin Poissant Feb 03 '16 at 16:20
@DustinPoissant In short, you're wrong. Run Felix's example and you'll see that you're wrong. – Mike Cluck Feb 03 '16 at 16:21
*"That depends where you call it"* I said `this` inside `bar`. Let me make it more concrete for you: `var foo = {bar: function bar() {console.log(this === bar); console.log(this === foo); }}; foo.bar();` You will hopefully see that if called this way, `this` refers to `foo`. – Felix Kling Feb 03 '16 at 16:21
I think we are referering to different "this`s you are referring to the this that is used to define the sibilings of each method. I am referring to the this within the function members – Dustin Poissant Feb 03 '16 at 16:22
1

I'm talking about `this` inside `bar`. There is only one of them. Point is that in order for `this` to refer to a function you either have to: a) Call a method on a function objects, such as in `(function() {}).bind()`. It's less common to add new methods to a function. b) Explicitly set `this` to a function object via `.call` or `.apply`. – Felix Kling Feb 03 '16 at 16:22
Well the error message you get `this.getNewPoint` is not defined, that is the `this` I am referring too. There is a reason that is not defined, because im right – Dustin Poissant Feb 03 '16 at 16:24
1

The reason why `this.getNewPoint` is not defined is because `this` refers to the global object, `window`, not to a specific function. And why is that? Because `forEach` calls the callback in such a way that `this` refers to `window`. I'm done now. If you don't want to believe me, then that's fine for me. I tried. – Felix Kling Feb 03 '16 at 16:26
@DustinPoissant [Look](https://jsfiddle.net/hwwmhd54/1/), `this` refers to the global scope, not the function. Simple. – Mike Cluck Feb 03 '16 at 16:27
I think you need to do some more research on how `this` works – Dustin Poissant Feb 03 '16 at 16:27
Your example only works because the `forEach` method passes the arrays `this` context into the function parameter – Dustin Poissant Feb 03 '16 at 16:30
1

@DustinPoissant Sure doesn't. [Here's an updated example](https://jsfiddle.net/hwwmhd54/2/) that doesn't use `forEach` at all. – Mike Cluck Feb 03 '16 at 16:31
2

That fact that you propose `this.getNewPoint.call(this, ...)` as solution demonstrates that you don't understand `this`. `foo.bar()` and `foo.bar.call(foo)` are **always** identical. – Felix Kling Feb 03 '16 at 16:31
I suggest you take your own advice. – Felix Kling Feb 03 '16 at 16:33
2

@DustinPoissant Prove to me how you're right. Write up a JSFiddle and if you can prove that Felix or I don't properly understand `this` then I will delete my Stack Overflow account. – Mike Cluck Feb 03 '16 at 16:33

Javascript and Callbacks

3 Answers3