How to call functions stored in an array

[piotr]

Hi,

I'd appreciate help with this issue. I have this code that generates error "Uncaught TypeError: command.val is not a function" whenever I try to call this.processCommands() with any of the keys as argument. What am I doing wrong?

 

processCommands: function(val) {
   var command = {
        'jump': this.jump,
        'run': this.run
    };
    command['val']();
},

jump: function() {
//code for jumping
},

run: function() {
//code for running
}

 

[Flip120]

Remove the quotes ---> command[val]()

[sapptime]

Shouldn't it be

command[val]();

The way you're doing it above uses the string 'val' instead of the parameter you're passing.

[Sebi]

As sapptime said, it should be:

command[val]();

However, if you call the functions like that, then they are called in a different context. So you might want to change that to:

command[val].call(this);

 

[piotr]

@sapptime yep, you are right. I shouldn't use quotes.

@SebastianNette yep, it worked thanks!

So to call this.player.run(); in player.js, I'd need command[val].call(this.player), right?

So, if functions have different contexts (or need to pass arguments) this doesn't make sense, becuase I'll need different ways to call them:

command = {
	rain: 	this.startRain, //in the current file
	jump: 	this.player.jumpUp, //jumpUp is in player.js
	shoot: 	this.singleBullet.fire(this.player)//needs an argument
};

That means I need to go back and use this, right?

if(val == 'rain') {
   this.startRain();
}

if(val == 'jump') {
   this.player.jumpUp();
}

if(val == 'shoot') {
   this.singleBullet.fire(this.player);
}

 

[Sebi]

If you need to call methods with arguments, then you could use apply for that.

command[val].apply(this, Array.prototype.slice.call(arguments, 1));
//...
processCommands("shoot", "first argument of the shoot method");

If you want to use a different context per function, you could bind the methods first:

command = {
	rain: 	this.startRain.bind(this), //in the current file
	jump: 	this.player.jumpUp.bind(this.player), //jumpUp is in player.js
	shoot: 	this.singleBullet.bind(this.singleBullet, this.player)//needs an argument
};
commad[val]();

There are dozens of ways to get that kind of stuff done.

Also I would move the command object outside of the processCommands method, or else it will be recreated everytime you use that method.

However, I would just remove that  processCommands function and just call the methods directly when needed. I don't see the benefit of using this.processCommands("jump") over this.player.jumpUp();

[mattstyles]

Just to save yourself a ton of headaches further down the line, make sure you heavily document when you are changing the scope for objects. I'm in the 'this' is evil camp but if I had an object full of methods I would expect 'this' to be equal to the object for each of those methods, the only exception would be event handlers (DOM events mutate scope which is standard JS/DOM behaviour) but even then I'd be inclined to say that if they are part of an object `this` should still be the object (this is against spec but the norm for most other classical languages and many JS frameworks also take this approach to have parity with how classical objects work).

Onboarding other devs would be a nightmare when scope changes frequently, this might not be an issue for you but if your project is non-trivial you will come back to this area of code at some point in the future and you will have forgotten all these scope changes. At least documentation will give you a heads up as to what to expect, note that documentation enforces nothing but at least you'll get a clue. Conventions and good documentation can help stop a 10 minute fix from ballooning into a one hour fix as you try and work out what on Earth you were coding in the first place, note again that there are better methods for this.

I'm a fan of the command pattern (you might call this a facade) but I'm not immediately clear why you're using it here, looks like you maybe want to reuse functions, which is a superb thing to do, but linking them to an object and introducing `this` (scope) everywhere stops that. Maybe you'd be better off using some sort of implementation of a mixin to drop into your classes? Or even higher-order classes that wrap a class/function and return another to use.

[piotr]

Thank you all for the replies. I apologize for not giving more context. As @mattstyles says, my goal is to reuse functions. E.g. to move the player, the game may accept from the user keywords such as "run", "move", "walk", "forward". The user input keywords in a text field, a function processes them, and check them against this dictionary of synonyms that map to specific functions. 

command = {
	'run': this.player.run,
   	'move': this.player.run,
   	'walk': this.player.run,
   	'forward': this.player.run,

   	'shoot': this.singleBullet.fire(this.player),
   	'kill': this.singleBullet.fire(this.player),
   	'bang!': this.singleBullet.fire(this.player),
   	'pewpew': this.singleBullet.fire(this.player)
};

 

[mattstyles]

There will be an issue with your `fire` functions there, as I think you want them executed when you call `command.shoot` but in your example they are triggered when that `command` object is created.

Things like .bind return a function which can be executed later, unless that `.fire` function returns a function you'll run into issues.

There are a couple of ways to fix it:

// Use an arrow function
// Arrow functions have no scope so they pinch it from above
// * Arrow scope is much trickier than it seems, and I have not tested this and 
//   scope can be mutated in lots of funky ways with JS
// * Arrows aren't quite ubiquitous so check which browsers you are supported

command = {
  shoot: () => this.bullet.fire( this.player )
}

// Thunkify
// This has (at least) 2 very pleasing extras:
// 1) The thunk function can be totally separate and reused
// 2) Allows you to call `command.shoot` with extra options

command = {
  shoot: function shootCache( bullet, player ) {
    // The params event and opts are totally optional and are only needed
    // if you want to pass through when you call `command.shoot`
    return function shoot( event, opts ) {
      bullet.fire( player )
    }
  }( this.bullet, this.player )
}

// Wrap in a function and bind scope

command = {
  shoot: function shoot() {
    this.bullet.fire( this.player )
  }.bind( this )
}

My personal favourite is to use thunks, but its largely dependent on how you want to manage things and the exact requirements of what you want to achieve.

Definitely have a look at mixins, I'm not really a fan of classes, inheritance and `this` in JS but it sounds like the mixin pattern would maybe help you out in adding reusable functionality to your objects.

 

[piotr]

@mattstyles yes, .fire is a function, and should be executed only on 'command.shoot', but in my example it was indeed executed on creation.

Anyways, thanks a lot for all those solutions! I've never heard of .bind, thunks or mixins, I'm going to give'em a try and learn something new!