1

Issue Resolved, any comments on best practice would be welcome

I'm working on a JS (and JQuery) Battleships game and have function 'dropBoat' that allows a user to place a ship on a board. The details of this function are not important (it takes board and boatLength as arguments) however I'm unable to reuse it to place a second boat.

I'm trying to achieve this chain of events:

dropBoat function called --> dropBoat function called --> dropBoat function called --> StartPlay function called. Here's a sketch of my current attempt.

var dropBoat = function(board, length, callback){
    //function code here
    $(document).keydown(function(e){
        if(e.which == "38"){
            //more function code here
            callback()
        }
    }
}

var dropThirdBoat = dropBoat(board1, 3, startPlay);
var dropSecondBoat = dropBoat(board1, 2, dropThirdBoat);
var dropFirstBoat = dropBoat(board1, 3, dropSecondBoat)();

this seems to work

var dropThirdBoat = function(){
    dropBoat(board1, 3, startPlay);
}
var dropSecondBoat = function(){
    dropBoat(board1, 2, dropThirdBoat);
}
var dropFirstBoat = dropBoat(board1, 3, dropSecondBoat);
    var dropBoat = function(board, length, callback){
    //function code here
    $(document).keydown(function(e){
        if(e.which == "38"){
            //more function code here
            callback();
        }
    }
}
oli5679
  • 1,709
  • 1
  • 22
  • 34
  • This code doesn't make a lot of sense, I'm sorry to say it. – Tomalak Apr 10 '15 at 06:39
  • Ah yeah I didn't want to get bogged down in explaining how I show outlines, rotate and place boats of different lengths according to key presses. I could well be doing lots of things in eccentric/inefficient ways as I'm quite new to programming. – oli5679 Apr 10 '15 at 06:53
  • That's no problem. Also good that you tried to cut down on the irrelevant parts instead of dropping your entire code, like many other newcomers to SO do. Showing your code is important, but explaining your intent is even more important. Your question is an example of [the XY problem](http://meta.stackexchange.com/questions/66377/what-is-the-xy-problem). – Tomalak Apr 10 '15 at 06:56

3 Answers3

1

You don't want to bind event handlers multiple times. Move the keydown handler out of your function code and to the top level.

$(document).keydown(function (e) {
    if (e.which == "38"){
        //more function code here
        callback();
    }
}

That being said, how about simply using a loop?

var dropBoat = function(board, length){
    //function code here
}

var board1 = new BattleshipBoard(),
    board2 = new BattleshipBoard(),
    boats = [1, 1, 1, 2, 2, 3, 5],
    b;

for (b = 0; b < boats.length; b++) {
    dropBoat(board1, boats[b]);
    dropBoat(board2, boats[b]);
}
Tomalak
  • 332,285
  • 67
  • 532
  • 628
  • Thank you I will try and improve on my use of event handlers. You're right that a loop would be a much simpler solution but I wanted users to have a more interactive experience by placing the boat with keyboard arrow keys (sorry I did not specify this in the question). – oli5679 Apr 10 '15 at 06:42
  • You can do that. Just call `dropBoat()` in the keyup handler. `dropBoat()` really never needs to call itself. – Tomalak Apr 10 '15 at 06:44
1

You need to declare your callback as an argument and you can call it directly using the argument name.

function doSomething(callback) {
     // ...

    // Call the callback
    callback('stuff', 'goes', 'here');
}

function foo(a, b, c) {
    // your operations
    alert(a + " " + b + " " + c);
}

doSomething(foo);

like this u will need to call your callback function.

Check this answer for more details :

https://stackoverflow.com/a/2190872/4763053

Community
  • 1
  • 1
Zealous System
  • 2,080
  • 11
  • 22
0

this seems to work

var dropThirdBoat = function(){
    dropBoat(board1, 3, startPlay);
}
var dropSecondBoat = function(){
    dropBoat(board1, 2, dropThirdBoat);
}
var dropFirstBoat = dropBoat(board1, 3, dropSecondBoat);
var dropBoat = function(board, length, callback){
   var storeCall = storeVal(callback);
   //function code here
    $(document).keydown(function(e){
        if(e.which == "38"){
            //more function code here
            storeCall()();
        }
    }
}

defining functions rather than variables stops the code from executing immediately.

oli5679
  • 1,709
  • 1
  • 22
  • 34