0

This is my code -

function searchString(usrLogin) {

  var setUsrLogin = function (usrLogin) {
    this.usrLogin = (usrLogin == "") ? "*" : usrLogin;
  }


  this.toString = function(){
    return 'source="dbmon-dump://Source/ID" ' + 'USR_LOGIN="' + this.usrLogin + '" ';
  }

  setUsrLogin(usrLogin);
}

$(function() {
  var a = new searchString("");
  $('#searchBar').val(a.toString());
});

a.toString() prints source="dbmon-dump://Source/ID" USR_LOGIN="undefined" because this.usrLogin shows up as undefined. What am I doing wrong?

Narabhut
  • 839
  • 4
  • 13
  • 30
  • You seem to be making the assumption that variables in functions and properties of `this` in the same functions are the same thing. Did you learn that somewhere? They're entirely separate. There are different solutions, but I'm not sure why you have these nested function in the first place. –  Jul 10 '13 at 13:41
  • I come from a Java background so that's a mistake on my part. – Narabhut Jul 10 '13 at 13:43
  • Ok, to use a solution most closely resembling your current code, you could change `var setUsrLogin = func...` to `this.setUsrLogin = func...`, then change the invocation to `this.setUsrLogin(usrLogin)`. But overall there seems to be more functions here than needed. –  Jul 10 '13 at 13:44
  • Crazy Train is right You are using private varaibles of function as the method of instance object. this.usrLogin is an instance property where as userLogin is the variable – chetan mehta Jul 10 '13 at 13:45
  • One more Thing in nested function Definitions inner function can not access the argument and this object of outer function so prob the issue too – chetan mehta Jul 10 '13 at 13:48

5 Answers5

6

When you call setUsrLogin(usrLogin);, your're losing the this context. In fact, the this on which you assign the usrLogin property on is the global object.

To fix this,

  • just remove that function call and set it directly:

    function searchString(usrLogin) {
        this.usrLogin = (usrLogin == "") ? "*" : usrLogin;
        this.toString = function(){
            return 'source="dbmon-dump://Source/ID" ' + 'USR_LOGIN="' + this.usrLogin + '" ';
        }
    }
    
  • or pass the instance as parameter. Either

    function setUsrLogin(usrLogin) {
        this.usrLogin = (usrLogin == "") ? "*" : usrLogin;
    }
    // and then
    setUsrLogin.call(this, usrLogin);
    

    or

    function setUsrLogin(search, usrLogin) {
        search.usrLogin = (usrLogin == "") ? "*" : usrLogin;
    }
    // and then
    setUsrLogin(this, usrLogin);
    

Btw, you might move the toString function to the prototype object:

function SearchString(usrLogin) {
   this.usrLogin = (usrLogin == "") ? "*" : usrLogin;
}
SearchString.prototype.toString = function() {
    return 'source="dbmon-dump://Source/ID" ' + 'USR_LOGIN="' + this.usrLogin + '" ';
};
var a = new SearchString("");
Community
  • 1
  • 1
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
0

The problem is that within your setUsrLogin function, this does not refer to the searchString instance.

What you can do is store a reference to your searchString function (which is definitely NOT a class) and use that to access the properties. Also, you could alter your setUsrLogin function to be like your toString function, which will then allow you to correctly access the searchString function using this.

This is one approach that will work:

function searchString(usrLogin) {
    var self = this;

    this.setUsrLogin = function (usrLogin) {
        self.usrLogin = (usrLogin == "") ? "*" : usrLogin;
    }

    this.toString = function () {
        return 'source="dbmon-dump://Source/ID" ' + 'USR_LOGIN="' + self.usrLogin + '" ';
    }

    self.setUsrLogin(usrLogin);
}

Here is a working example

musefan
  • 47,875
  • 21
  • 135
  • 185
  • @Bergi: No it does not, what? – musefan Jul 10 '13 at 13:50
  • 1
    stop using term class in javascipt. Use reference Type instead – chetan mehta Jul 10 '13 at 13:50
  • 1
    *"`this` refers to the `toString` function"* It doesn't. –  Jul 10 '13 at 13:52
  • @chetanmehta: OP used term "class" that is why I used it, so they understood what I was referring to – musefan Jul 10 '13 at 13:52
  • Your code works but your reason why is wrong. http://jsfiddle.net/LLXTj/1/ `this` is just fine in the `toString` function. The change you made in `setUsrLogin` is what fixes it. – James Montagne Jul 10 '13 at 13:53
  • *"`this` does not refer to the `searchString` instance."* Actually, it does since OP is doing `a.toString()`. –  Jul 10 '13 at 13:56
  • What do you mean with _"Also, you could alter your setUsrLogin function to be like your toString function, which will then allow you to correctly access the searchString function using this."_? I dont think that is an alternative way of solution. – Esteban Jul 10 '13 at 14:23
  • @ESt3b4n: If you decalre the funtion as `this.setUsrLogin` then you can set the value using `this.usrLogin =` and it will work. Which is alternate to what I have provided – musefan Jul 10 '13 at 14:35
  • No, it's not about having a bad day, or about bad behavior. It's simply that you're conveying a fundamental misunderstanding of the problem, as I described [here](http://stackoverflow.com/questions/17572101/undefined-class-variables-in-javascript#comment25566978_17572357). It's funny that people complain when they don't get a comment explaining a downvote, yet they accuse when they do get their explanation. –  Jul 10 '13 at 14:41
  • @CrazyTrain: Actually, you are the only one that has bothered to explain anything, and I edited my answer accordingly each time. Fact is other way around, based on there being a working solution I would have held off on a downvote and instead left a comment t give the ser a chance to correct, as they were pretty much there just needed a slight edit – musefan Jul 10 '13 at 14:45
  • @musefan: Ah, you mean like the answer I just posted? mmm, you forgot to mention the way it should be called (this.setUsrLogin(usrLogin); instead of setUsrLogin(usrLogin);) Please try to post well understand answer. – Esteban Jul 10 '13 at 14:47
  • Well, I don't think that multiple comments describing the same issue are needed, but yes, you're right, I see now that you changed the function name, though you're still unnecessarily using `self` in `toString`. Not that it's wrong, just unnecessary unless OP wants it permanently bound. –  Jul 10 '13 at 14:47
  • @CrazyTrain: True, comments don't need to say the same thing (hence the ability to upvote) but I cannot imagine the first downvoters were waiting around for you to come along and explain the issue just so they could +1 your comment – musefan Jul 10 '13 at 14:49
  • 1
    @musefan: Hmmm... I tend to assume that people *are* waiting around to hear what I have to say. ;D –  Jul 10 '13 at 14:52
  • @CrazyTrain: Agree with the `self` part. This is because I headed down one road to fix the issue (adding the `self`), and just so happened to be able to fix it another way at the same time, so they colided – musefan Jul 10 '13 at 14:52
  • wow with so many "edit"s of your answer, you are messing everything.... you are simple putting 2 ways of solutions in one :S 1) using a variable holding "this" 2) using "this.setUsrLogin = function {..." and calling it "this.setUsrLogin(usrLogin);", but you put self??? you are doing 2 solutions in one, and that WILL confuse anybody new in javascript!!!! – Esteban Jul 10 '13 at 14:58
  • @ESt3b4n: 2 solutions to the problem yes, but as it stands it is not incorrect. This exact code may not need `self` but there are plenty of situations where the use of `self` can be very valuable, even along with the `this.method` solution. For example, if you have something else within the `toString` function that redifines `this`, then you can only access the original propertis with `self` – musefan Jul 10 '13 at 15:02
  • Just see my post, it´s almost the same, but you have added an unnecesary variable "self" (you are mixing @prasun answer an mine). For a newber, you will get him more confuse with you so-edited answer. Plz, in the future post an answer well thought at first, so you may avoid editions, just a constructive suggestion. Have a nice day! – Esteban Jul 10 '13 at 15:06
  • @ESt3b4n: I think you are missing my point. Also, the edit feature is there for a reason. As my code stands, tell me what is wrong with it? Not from an OP point of view, but why I can't use `self` just because I don't need it? – musefan Jul 10 '13 at 15:08
0
function searchString(usrLogin) {
    this.usrLogin = (usrLogin == "") ? "*" : usrLogin;
    this.toString = function(){
    return 'source="dbmon-dump://Source/ID" ' + 'USR_LOGIN="' + this.usrLogin + '" ';
      };
}


  var a = new searchString("abc");
  alert(a.toString());

This will alert the string with a username, let me know if this helps or isn't what you are looking for. http://jsfiddle.net/rBe6x/

munch1324
  • 1,148
  • 5
  • 10
0

Bergi's answer pretty much covers it.

Adding another way of achieving it - using a variable to save the context of this

function searchString(usrLogin) {
  //save the context
  var that = this;
  var setUsrLogin = function (usrLogin) {
     //use that in the internal functions/methods
     that.usrLogin = (usrLogin == "") ? "*" : usrLogin;
  }


  this.toString = function(){
      //use that in the internal functions/methods [Not required in this case though]
      return 'source="dbmon-dump://Source/ID" ' + 'USR_LOGIN="' + that.usrLogin + '" ';
  }

  setUsrLogin(usrLogin);
}
prasun
  • 7,073
  • 9
  • 41
  • 59
  • 1
    You dont need to pass "that" into the toString() function. Just leave it as "this". Use "that", only for the setUsrLogin() function. – Esteban Jul 10 '13 at 14:17
  • @ESt3b4n agreed, but if it is there it has no harm, it may avoid chances of human mistakes. – prasun Jul 10 '13 at 14:45
  • I am the one who try to avoid using variables to save the context of this, if there are other possible ways to achieve the problem, because it make generate a little confusion of why create/repeat the same thing in another variable... I would use it as a last resource. – Esteban Jul 10 '13 at 14:53
  • @ESt3b4n I have added that in comment, that not required though, let the developer use that option as per the use case. Thanks for comments – prasun Jul 10 '13 at 15:02
0

I agree with Bergi answer.

Here is another "simpler" solution:


    function searchString(usrLogin) {

      this.setUsrLogin = function (usrLogin) {
         this.usrLogin = (usrLogin == "") ? "*" : usrLogin;
      }

      this.toString = function(){
          return 'source="dbmon-dump://Source/ID" ' + 'USR_LOGIN="' + this.usrLogin + '" ';
      }

      this.setUsrLogin(usrLogin);
    }






Esteban
  • 162
  • 8