0

Problem

Uncaught TypeError: state.removeFlag is not a function

Extra Information

I'm having trouble with the code below, giving me the error above. My intention is simple, I want to set up two functions in a 'class' then call them, but I am getting the above error and I can't spot the problem. I was hoping that another pair of eyes might help. I appreciate any help you can give me.

Note: Flags.CROSS_SLOT has a value of 1

Code

function State(){
    this.state = 0; //bitstring representing the user's choices in the UI

    this.addFlag = function(flag){
        state = (state | flag);
    }

    this.removeFlag = function(flag){
        state = (~state | flag);
    }
}

var state = new State();

state.addFlag(Flags.CROSS_SLOT);
console.log(state.state);
state.removeFlag(Flags.CROSS_SLOT);
console.log(state.state);
shermanzach
  • 581
  • 1
  • 6
  • 14

4 Answers4

2

You need to use the this keyword inside your functions. Also, I personally prefer to move function outside the object and use prototype...

function State(){
    this.state = 0; //bitstring representing the user's choices in the UI
}

State.prototype.addFlag = function(flag){
    this.state = (this.state | flag);
}

State.prototype.removeFlag = function(flag){
    this.state = (~this.state | flag);
}
Shawn Jacobson
  • 1,272
  • 8
  • 12
2

You're overwriting the state variable

this.addFlag = function(flag){
    state = (state | flag); // that's the global variable defined outside
}

var state = new State();

the state variable, is no longer a new instance of the function, but 0 instead, you need to access this.state instead

this.addFlag = function(flag){
    this.state = (state | flag); // that's the global variable defined outside
}
adeneo
  • 293,187
  • 26
  • 361
  • 361
2

The problem is that the methods in the class are changing the global state variable that you used to hold the instance of the class, instead of changing the property in the class. The first call overwrites the variable, so the second call fails as the variable no longer contains the object.

Javascript doesn't have object scope, so using the state identifier inside a method doesn't mean the state property in the object. You need to use the this keyword to access the members in the object.

Also, use the & operator ro remove a flag and apply the ~ operator to the flag, otherwise you will flip all flags and add the flag that you tried to remove.

function State(){
    this.state = 0; //bitstring representing the user's choices in the UI

    this.addFlag = function(flag){
        this.state = this.state | flag;
    }

    this.removeFlag = function(flag){
        this.state = this.state & ~flag;
    }
}

Demo:

// function to show values in StackOverflow snippet
function log(s) { document.write(s + '<br>'); };


function State(){
    this.state = 0; //bitstring representing the user's choices in the UI

    this.addFlag = function(flag){
        this.state = this.state | flag;
    }

    this.removeFlag = function(flag){
        this.state = this.state & ~flag;
    }
}

var state = new State();

var Flags = {
  CROSS_SLOT: 1
};

state.addFlag(Flags.CROSS_SLOT);
log(state.state);
state.removeFlag(Flags.CROSS_SLOT);
log(state.state);
Guffa
  • 640,220
  • 96
  • 678
  • 956
  • Additionally, 'this' may not always be what you intend. It is safer, and common practice to define it inside the function itself: var self = this; Then use self everywhere where this was was used. http://stackoverflow.com/questions/337878/var-self-this – Chris Clayton May 06 '15 at 18:57
  • @razethestray: That is one way to handle the context, but I wouldn't say that it's common practice. – Guffa May 06 '15 at 19:13
  • @Guffa Thanks for the hint about the bitwise removal, I was on my way to testing my version but I ran into this error, which blocked me. – shermanzach May 06 '15 at 19:34
1

you could try something like this:

function State(){
    return  {
        state: 0,
        addFlag: function(flag){
            this.state = (this.state | flag);
        },
        removeFlag: function(flag){
            this.state = (~this.state | flag);
        }
    }
}
Vee6
  • 1,344
  • 2
  • 18
  • 34