Handling keyboard shortcuts for webapp

Question

Please review the code.

document.addEventListener("keydown", function(e) { // shortcuts
 if (e.ctrlKey) { // Ctrl+
 if (/^(82|79|83|66|191)$/.test(e.keyCode)) {
 e.preventDefault();
 }
 switch (e.keyCode) {
 case 82: // R
 newDoc();
 break;
 case 79: // O
 openDoc();
 break;
 case 83: // S
 saveDoc();
 break;
 case 66: // B
 showHideStatusBar(statusBarOn ? false : true); // toggle
 break;
 case 191: // /
 alert("Welcome to " + appname + "!");
 break;
 }
 }
 if (e.keyCode == 9) { // tab
 e.preventDefault();
 var sStart = textarea.selectionStart,
 text = textarea.value;
 textarea.value = text.substring(0, sStart) + "\t" + text.substring(textarea.selectionEnd);
 textarea.selectionEnd = sStart + 1;
 }
});

• Anirban Nag 'tintinmj': What's the background/usage of your app? On what basis you want to review your code? readability/clean-code/optimization? Please mention them.
• user31946: optimization and simplifying mainly... it is for a text-editing webapp..

Answer 1

In JS, when you have a switch, it's usually better to go with a dispatch table.

var mapping = {
 82: newDoc,
 79: openDoc,
 83: saveDoc,
 66: function() {
 showHideStatusBar(statusBarOn ? false : true);
 },
 191: function() {
 alert("Welcome to " + appname + "!");
 }
};
mapping[e.keyCode]();

Or if you don't want to give a name:

({
 82: newDoc,
 79: openDoc,
 83: saveDoc,
 66: function() {
 showHideStatusBar(statusBarOn ? false : true);
 },
 191: function() {
 alert("Welcome to " + appname + "!");
 }
})[e.keyCode]();

Also, use === instead of == for multiple reasons. Google "triple equal javascript" if you want more information.

• amon: It's generally a good idea to use a dispatch table instead of a switch. However, you have to handle a default case during lookup, something like var fn = mapping[e.keyCode]; if (fn !== undefined) { fn() } else { default() }
• Florian Margaine: There is no default case in his code.
• Florian Margaine: Except there is an if preventing this :)
• amon: /me crawls back in shame for not reading the code carefully... thanks.