could someone check over this snippet of javascript please

jamesmorris86 · 12 Jun 2015 at 21:17

Hey guys, I have this bit of javascript which loads a certain url if the w,a,s,d keys are pressed. Its working fine but a lot of examples I've been looking at include things like break; and addeventlistener.

Mine does not include these things so I just wanted to check my code isn't incomplete or flawed. Thanks guys.

Code:

 <script>
    document.onkeydown = function(e) {
    e = e || window.event;
    key = e.keyCode || e.charCode;
    var keys = { 
      87: '?move=up', 
      68: '?move=right', 
      83: ?move=down', 
      65: '?move=left'
    };
    if (keys[key]) window.location.href = keys[key];
    };
    </script>

xbenjiiman · 12 Jun 2015 at 23:22

It's pretty much another way of writing

Code:

document.addEventListener('keydown', function(){});

Otherwise it looks fine. There's no logical reason to use break here since you're not looping.

jamesmorris86 · 12 Jun 2015 at 23:47

Cool, thanks.

Should I add the line you have written then ?

xbenjiiman · 13 Jun 2015 at 19:40

It's a matter of preference, onkeydown uses addEventListener under the hood. addEventListener is the most popular of the two though.

jamesmorris86 · 13 Jun 2015 at 21:10

Okay, Sorry to be a pain, how would I implement it into my bit of code.
I know next to nothing with javascript so have been trying a few different ways but they all seem to break the code. Firefoxs debugger isn't showing me any error messages either so I'm at a loss.

Thanks again for all your help.

touch · 13 Jun 2015 at 21:26

This line:

document.onkeydown = function(e) {

Could be changed for the code that xbenjiman posted:

document.addEventListener('keydown', function(e){

There's no real reason that you would need to change it in your code - they essentially do the same thing.

jamesmorris86 · 13 Jun 2015 at 21:34

Thanks, I replaced that line and changed the bottom }; to }); and its working now