0

I am attempting to make a project which does two way data binding on two specified variables. However, when I tried it out, the project did not seem to be working.

I'm pretty sure that what I did wrong is that I specified a constructor, and then inside, I created a variable using "this," a function using "this," and then I tried to use the first variable inside the function using "this." Is this allowed?

The code for my project is in the snippet below. 

function glue(varOne, varTwo, interval = 60) {
  this.varOne = varOne;
  this.varTwo = varTwo;
  this.varOneClone = this.varOne;
  this.varTwoClone = this.varTwo;
  this.interval = interval;
  this.onChange = function(changedVar) {
    if (changedVar == this.varOne) {
      this.varTwo = this.varOne;
    } else if (changedVar == this.varTwo) {
      this.varOne = this.varTwo;
    }
    this.varOneClone = this.varOne;
    this.varTwoClone = this.varTwo;
  };
  this.intervalID = setInterval(function() {
    if (this.varOne != this.varTwo) {
      if (this.varOne != this.varOneClone) {
        this.onChange(this.varOne);
      } else if (this.varTwo != this.varTwoClone) {
        this.onChange(this.varTwo);
      }
    }
  }, this.interval);
  this.clearUpdate = function() {
    clearInterval(intervalID);
  };
  this.changeUpdate = function(newInterval) {
    this.interval = newInterval;
    clearInterval(intervalID);
    this.intervalID = setInterval(function() {
      if (this.varOne != this.varTwo) {
        if (this.varOne != this.varOneClone) {
          this.onChange(this.varOne);
        } else if (this.varTwo != this.varTwoClone) {
          this.onChange(this.varTwo);
        }
      }
    }, this.interval);
  };
}
<!DOCTYPE html>
<html>

<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width">
  <title>Glue</title>
</head>

<body>
  <input id="input" type="text"></input>
  <p id="output"></p>
  <script>
    var input = document.getElementById("input");
    var output = document.getElementById("ouput");
    var glue = new glue(input, output, 60);
  </script>
</body>

</html>

Thank you!

Edit:

I've tried using the var self = this; method which two people recommended, but it still refuses to work. The error from the console is TypeError: glue is not a constructor, but I'm not sure why this happens. I want glue to be a constructor. Please help! The new code is below.

function glue(varOne, varTwo, interval = 60) {
  var self = this;
  self.varOne = varOne;
  self.varTwo = varTwo;
  self.varOneClone = self.varOne;
  self.varTwoClone = self.varTwo;
  self.interval = interval;
  self.onChange = function(changedVar) {
    if (changedVar == self.varOne) {
      self.varTwo = self.varOne;
    } else if (changedVar == self.varTwo) {
      self.varOne = self.varTwo;
    }
    self.varOneClone = self.varOne;
    self.varTwoClone = self.varTwo;
  };
  self.intervalID = setInterval(function() {
    if (self.varOne != self.varTwo) {
      if (self.varOne != self.varOneClone) {
        self.onChange(self.varOne);
      } else if (self.varTwo != self.varTwoClone) {
        self.onChange(self.varTwo);
      }
    }
  }, self.interval);
  self.clearUpdate = function() {
    clearInterval(intervalID);
  };
  self.changeUpdate = function(newInterval) {
    self.interval = newInterval;
    clearInterval(intervalID);
    self.intervalID = setInterval(function() {
      if (self.varOne != self.varTwo) {
        if (self.varOne != self.varOneClone) {
          self.onChange(self.varOne);
        } else if (self.varTwo != self.varTwoClone) {
          self.onChange(self.varTwo);
        }
      }
    }, self.interval);
  };
}
<!DOCTYPE html>
<html>

<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width">
  <title>Glue</title>
</head>

<body>
  <input id="input" type="text"></input>
  <p id="output"></p>
  <script>
    var input = document.getElementById("input");
    var output = document.getElementById("ouput");
    var glue = new glue(input, output, 60);
  </script>
</body>

</html>

Thanks for your help!

Fluzz
  • 68
  • 12
  • BTW, you should really give this question a better title. The title should summarize the specific problem. In most cases, there's not a need for the title to reference what the goal of your overall project is. [This Help Center topic](https://stackoverflow.com/help/how-to-ask) has some good tips on what makes a good title. – Elezar Jan 26 '16 at 04:21
  • All right, I'll look at that article. I'll also try your second suggestion, which Loïc Faure-Lacroix also recommended. Thanks! – Fluzz Jan 26 '16 at 04:25
  • Oh, it looks like you're not even getting to the problem that we were trying to solve with `this`! This problem is probably because you're using `glue` both for the name of your object/constructor, and as the name of the variable that you're trying to assign the object to. Without knowing where you're defining the constructor, I can't say for sure, but that's likely the problem. – Elezar Jan 26 '16 at 04:44
  • http://stackoverflow.com/questions/591269/settimeout-and-this-in-javascript – epascarello Jan 26 '16 at 04:45
  • Actually, are you referencing the `glue` constructor anywhere? That html snippet is an entire page, but it doesn't include the JS from your first snippet, and also isn't referencing an external JS file... Is that just something that's missing from your snippets? – Elezar Jan 26 '16 at 04:46
  • I'm not sure what you meant when you said I'm using the same name for the object and constructor. I changed the name of the instance of "glue" that I created: [http://jsbin.com/woyoju/edit?html,js,output](http://jsbin.com/woyoju/edit?html,js,output). To answer your second question (I think), I used a script tag in the body of my HTML. – Fluzz Jan 26 '16 at 04:55
  • Sorry it took so long to respond. I've been mostly away from the computer the past couple days. I've updated my answer below to include solutions to all of the problems I see in your code snippet & jsbin. – Elezar Jan 28 '16 at 16:35

2 Answers2

3

There are few problems that you could fix, as you said, you're using this everywhere. this might not always point to the same object in some context so if you want to use the same object, you could use a variable accessible from the scope.

Usually, people use a variable named self:

var self = this

Then the self variable should be used everywhere where you want to access specifically that object. So in your onChange and setInterval, it would be a better idea to access this by using the self object.

One other way, would be to instead bind a bounded function. In this case, the this object will be the one you bound the function to.

this.onChange = (function () {
  // here this will be equal to context
}).bind(context)

In this case, you can set context to this and within the function you can still use this without worrying which object will be this.

You can also make sure to call method using this.func.call(this, params...) or this.func.apply(this, paramArray).

There are a lot of ways to fix this, but if you search about bind, apply and call. It will be enough to understand how to make more complex construction for your code.

To give you an idea of how bind works, take this example:

function bind(method, obj) {
  // Create a callable
  return function () {
    // Pass the arguments to the method but use obj as this instead
    return method.apply(obj, arguments);
  }
}

function func(a, b, c) {
  console.log(this, a, b, c)
}

var bound1 = bind(func, {a: 1})
var bound2 = bind(func, {a: 2})

func(1,2,3)
bound1(3,4,5)
bound2(6,7,8)

You'll see that when func isn't called, this defaults to window. But in case of bound1 and bound2, it will be using the second parameter of bind.

All that to say that you can control which object will be used as this but if you don't specify which object is going to be used, then you might end up using window as this like in a setInterval. In other words, the keyword this isn't scoped like other variables. It depends of the context, not of its place in the code.

Loïc Faure-Lacroix
  • 13,220
  • 6
  • 67
  • 99
2

The problem is that the setInterval callback is made in a different context, so this will no longer be your glue object. Since you're running within a browser, it will be the window object. I'm pretty sure that all the other times you use this, it is referencing the correct object.

There are a few options to handle this. The first is to use the .bind() method. It is definitely the cleanest, and requires the least amount of "tweaking". However, it's not supported by IE8. Hopefully you don't have to support that browser, considering MS has dropped support for it, except for in embedded systems. Here's how it would work:

  this.intervalID = setInterval(function(self) {
    if (this.varOne != this.varTwo) {
      if (this.varOne != this.varOneClone) {
        this.onChange(this.varOne);
      } else if (this.varTwo != this.varTwoClone) {
        this.onChange(this.varTwo);
      }
    }
  }.bind(this), this.interval);

Another option is to create a variable that holds the value of this before you call setInterval and then rely on the closure to give you access to that variable:

function glue(varOne, varTwo, interval = 60) {
  var self = this;
  //...
  this.intervalID = setInterval(function() {
    if (self.varOne != self.varTwo) {
      if (self.varOne != self.varOneClone) {
        self.onChange(self.varOne);
      } else if (self.varTwo != self.varTwoClone) {
        self.onChange(self.varTwo);
      }
    }
  }, this.interval);
  //...
}

Finally, you could also use an immediately-invoked function to pass in the value of this without creating a closure:

  this.intervalID = setInterval((function(self) {
    return function() {
      if (self.varOne != self.varTwo) {
        if (self.varOne != self.varOneClone) {
          self.onChange(self.varOne);
        } else if (self.varTwo != self.varTwoClone) {
          self.onChange(self.varTwo);
        }
      }
    }
  })(this), this.interval);

EDIT:

Everything said above is a problem and the options given for solving it should fix that problem. However, it's one of only a few problems in your code sample.

There's also a problem with the line var glue = new glue(input, output, 60);. You can't have a variable with the same name as a function. If the function is declared first, as it should be in this case, then the variable overwrites it, so the function essentially no longer exists by the time you call new glue(). This is why you are getting the glue is not a constructor error.

I see that in your jsbin you've changed the variable name to tape. That fixes the problem. However, jsbin puts all the code from the JavaScript pane at the bottom of your body. You need the function to be declared before the var tape = new glue(input, output, 60); line, since that line is calling the function. You can tell jsbin where to put the code from the JS pane, by putting %code% where you want it in in the HTML pane. So, if you put a line like <script>%code%</script> before your existing script block, it should fix that. This is totally just a quirk of using jsbin, and won't apply to code you have running in a standalone website (although, even on your standalone site, you need to make sure that the code declaring the glue function comes before the code calling it).

Now, that gets rid of all the errors that are being thrown, but still, the code isn't actually DOING anything. This is because near the beginning of the constructor you have:

this.varOneClone = this.varOne;
this.varTwoClone = this.varTwo;

So varOne starts out equal to varOneClone, and varTwo starts out equal to varTwoClone. The only other place you set them is inside the onChange method, but you only call onChange if varOne != varOneClone or varTwo != varTwoClone. It's like you're saying "Make these 2 values the same, then if they're different, call onChange." Obviously, in that case, onChange is never going to be called.

I realize that it's possible you have more code than you've included here, that IS changing those properties, but I think your goal is to check if the text WITHIN varOne or varTwo has changed, and if so, update the other one, rather than checking if the elements themselves have changed. Since the text can be changed by the user (at least for varOne, since that's an input), it can be changed outside of code. If assumption is correct, you need something like this:

function glue(varOne, varTwo, interval = 60) {
  this.varOne = varOne;
  this.varTwo = varTwo;
  this.varOneCurrentText = this.varOne.value;
  this.varTwoCurrentText = this.varTwo.textContent;
  this.interval = interval;
  this.onChange = function(changedVar) {
    if (changedVar == this.varOne) {
      this.varTwo.textContent = this.varOneCurrentText = this.varTwoCurrentText = this.varOne.value;
    } else if (changedVar == this.varTwo) {
      this.varOne.value = this.varOneCurrentText = this.varTwoCurrentText = this.varTwo.textContent;
    }
  };
  this.intervalID = setInterval(function() {
    if (this.varOne.value != this.varTwo.textContent) {
      if (this.varOne.value != this.varOneCurrentText) {
        this.onChange(this.varOne);
      } else if (this.varTwo.textContent != this.varTwoCurrentText) {
        this.onChange(this.varTwo);
      }
    }
  }.bind(this), this.interval);
  //...
}

A couple things to note about this. First, notice that I'm using .value for varOne but .textContent for varTwo. This is because you're passing in a form element (an input) for varOne and a non-form element (a paragraph) for varTwo. These types of elements have different ways of getting their current text. If you can design it so that only form elements will ever be passed in, it will make things easier. But if not, since you probably won't know in advance what type of elements are passed in, you'd need to add a check at the beginning, so you can use the correct property.

Also, while this should work, it would really be better to use events rather than having a continuous loop in the setInterval looking to see if the text has changed. The input has a change event that will be fired anytime its value is changed. You could just update varTwo whenever that event is fired. I don't think there's a similar event for the paragraph element, but you could create a custom event for it. I'm assuming that you're planning on having some other code that will update the text inside the paragraph, since that's not something the user can do directly. If so, then you could fire your custom event at the same time that you update its text. Then when that event is fired, you could update varOne.

I just noticed a typo in a line you have, as well:

var output = document.getElementById("ouput");

The element ID should of course be "output".

Elezar
  • 1,467
  • 1
  • 15
  • 22
  • Your last example, doesn't work. If you call the method, it will not get called after `this.interval`. To do that, you'd have to return a method with the self bound. – Loïc Faure-Lacroix Jan 26 '16 at 04:29
  • Whoops, yeah. I was copy/pasting a lot of that, and forgot to add in the `return function()` part of it on that example. I've edited it to fix it. – Elezar Jan 26 '16 at 04:35
  • Wait: do you mean that I should use one of them, or all of them? You used your second solution in your third solution. – Fluzz Jan 26 '16 at 05:24
  • @Fluzz the third solution is different from the second in the sense it creates an extra closure containing the `self`, in other words if `self` was defined above, it would ghost it with whatever parameter is sent to the anonymous function. The third solution is equivalent to using `bind`. Except bind override `this` not a parameter. – Loïc Faure-Lacroix Jan 26 '16 at 08:46
  • I used the "self" method, but it still doesn't work: [http://jsbin.com/woyoju/4/edit?js,output](http://jsbin.com/woyoju/4/edit?js,output). – Fluzz Jan 27 '16 at 00:49
  • Oh my. You reaally put a lot of work into this. I really appreciate it! – Fluzz Feb 02 '16 at 06:36