JS, is this too much code?

jamesmorris86 · 1 Jan 2016 at 11:10

Hey guys. Happy new year!

I'm slowly getting my head around javascript, with a lot of help from this forum

I am using this code to listen out for touches which is working well now.
It seems like I am repeating myself quite a lot. Is there a better technique I could be using for the following?

Code:

function onload(){
    var left= document.getElementById('left')
        left.addEventListener('touchstart', function (){ move('left') }, false);
        left.addEventListener('touchend', function (){ move('left') }, false);

    var right= document.getElementById('right')
        right.addEventListener('touchstart', function (){ move('right') }, false);
        right.addEventListener('touchend', function (){ move('right') }, false);

    var up= document.getElementById('up')
        up.addEventListener('touchstart', function (){ move('up') }, false);
        up.addEventListener('touchend', function (){ move('up') }, false);

    var down= document.getElementById('down')
        down.addEventListener('touchstart', function (){ move('down') }, false);
        down.addEventListener('touchend', function (){ move('down') }, false);
    }

Thanks

Scotty123 · 1 Jan 2016 at 11:26

Could have it as a function where you pass the Id you're interested in, e.g. DoSomething('down').

Tripnologist · 4 Jan 2016 at 08:46

Sorry this is in noob-y jQuery, but should be easy enough to convert to vanilla.

Code:

function onload(){

    $('#left, #right, #up, #down').each(function(){
        var $this = $(this);        
        var id = $this.attr('id');
        $this.on('touchstart touchend', function(){
            move(id);
        });
        
    });

}

I'd personally use a data attribute rather than IDs though.

Code:

<div data-move="left"></div>

function onload(){

    $('[data-move]').on('touchstart touchend', function(){
        var direction = $(this).attr('data-move');
        move(direction);
    });

}

roccles · 4 Jan 2016 at 12:07

Something similar to below would reduce the amount of repetition.

Code:

function direction(d){
    var dir= document.getElementById(d);
    dir.addEventListener('touchstart', function (){ move(d) }, false);
    dir.addEventListener('touchend', function (){ move(d) }, false);
}

direction('left');
direction('right');
direction('up');
direction('down');

xbenjiiman · 4 Jan 2016 at 15:40

If you wanted to get fancy, you could do something like this.

Code:

function onload() {
	['left', 'right', 'up', 'down'].forEach(function(direction) {
		var el = document.getElementById(direction);
		el.addEventListener('touchstart', move.bind(null, direction), false);
		el.addEventListener('touchend', move.bind(null, direction), false);
	});
}

Or if you wanted to annoy your co-worker,

Code:

['touchstart', 'touchend'].forEach(listener => ['left', 'right', 'up', 'down'].forEach(direction => document.getElementById(direction).addEventListener(listener, move.bind(null, direction), false)));

A bit on arrow functions here: https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Functions/Arrow_functions

GravyMonster · 4 Jan 2016 at 17:04

Tripnologist's second example is what I'd use, although with the data() method to retrieve the value as it uses jQuery's internal cache instead of accessing the DOM again, and selecting by class as it's faster that the attribute selector.

Code:

<div class="move" data-direction="left"></div>

function onload(){
    $('.move').on('touchstart touchend', function() {  
        move($(this).data('direction'));
    });

}