3
\$\begingroup\$

I have some code which moves the users position up, right, down or left depending on the direction entered.

function up(field, player) {
    player.column = player.column + 1;
    field[player.row][player.column] = 'P'; 
}

function right(field, player) {
    player.row = player.row + 1;
    field[player.row][player.column] = 'P';
}

function down(field, player) {
    player.column = player.column - 1;
    field[player.row][player.column] = 'P';
}

function left(field, player) {
    player.row = player.row - 1;
    field[player.row][player.column] = 'P';
}

The code works fine for what I need, but I was curious if there is a more efficient way of writing the 4 functions above? As in maybe use one function rather than 4?

note Edge cases will be implemented - if the next position is greater than the dimensions of the field, etc. So I'm not sure if this will impact making it more efficient

\$\endgroup\$
4
\$\begingroup\$

How about

function move(field, player, dx, dy) {
    player.column = player.column + dx;
    player.row = player.row + dy;
    field[player.row][player.column] = 'P'; 
}

and instead of calling up(field, player), call move(field, player, 0, -1)? With your input, that would basically be

switch(input) {
case 'up':
    move(field, player, 0, -1);
    break;
case 'right':
    move(field, player, 1, 0);
    break;
case 'left':
    move(field, player, -1, 0);
    break;
case 'down':
    move(field, player, 0, 1);
    break;
}

I assume you'll have more code, like checking if the move is valid, resetting the field at the current player position, etc., which won't be duplicated this way. A more complete example might be

function move(field, player, dx, dy) {
    // Valid move?
    let newColumn = player.column + dx;
    let newRow = player.row + dy;
    if (newColumn < 0 || newColumn >= WIDTH ||
        newRow < 0 || newRow >= HEIGHT)
        return;
    // Player cannot hit wall
    if (field[newRow][newColumn] == 'X')
        return;

    // Make move
    field[player.row][player.column] = ' ';
    player.column = newColumn;
    player.row = newRow;
    field[player.row][player.column] = 'P'; 
}

Edge cases will be implemented - if the next position is greater than the dimensions of the field, etc. So I'm not sure if this will impact making it more efficient

It might seem unnecessary to verify newColumn when you're making a move in the y (row) direction, but comparisons like that are extremely cheap and you shouldn't be worried about their performance.

\$\endgroup\$
3
  • \$\begingroup\$ I like this a lot... But I'm not sure if it would work with the input. The input is an array of strings like up, right, right, down, east, east, west. So if west, - 1 on the the rows, +1 for east, etc. I may be not understanding your code, but I'm not sure how this function can interperate when to add + 1 for going up, -1 for going down, etc. Thanks though \$\endgroup\$ Jun 29 '19 at 11:46
  • 1
    \$\begingroup\$ See my updated answer for an example how to act on different input strings. \$\endgroup\$
    – Glorfindel
    Jun 29 '19 at 11:58
  • \$\begingroup\$ Thank you, it makes more sense now. Never considered using switch statements. \$\endgroup\$ Jun 29 '19 at 12:15
4
\$\begingroup\$

Your functions are called up, east, down, up. That's a strange combination of directions.

Either name them north, east, south, west. Or up, left, down, right.

Instead of saying variable = variable + 1, you can also say variable += 1, which is shorter and more to the point.

And for variable += 1 there is another abbreviation, which is variable++.

\$\endgroup\$
2
  • \$\begingroup\$ You're right. I'm not sure why I named them like that - in my production code it's like how you wrote it. Thanks for the heads up \$\endgroup\$ Jun 29 '19 at 11:47
  • \$\begingroup\$ Possible invalidation of this answer? \$\endgroup\$
    – dfhwze
    Jun 29 '19 at 13:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.