simplifying this block of javascript code (if)

Question

Please help me simplifying this block of code.

any idea on how can I combine the last two if statements while preserving the functionality same.

Thanks.

document.addEventListener("keydown", function(e) { // shortcuts
  var mapping = {
    "noctrl9": function() { // tab
      var sStart = textarea.selectionStart,
        text = textarea.value;
      textarea.value = text.substring(0, sStart) + "\t" + text.substring(textarea.selectionEnd);
      textarea.selectionEnd = sStart + 1;
    },
    66: function() { // B
      showHideStatusBar(statusBarOn ? false : true);
    },
    79: openDoc, // O
    82: newDoc, // R
    83: saveDoc, // S
    191: function() { // /
      alert("Welcome to " + appname + "!");
    }
  };
  if (e.ctrlKey && mapping[e.keyCode]) {
    e.preventDefault();
    mapping[e.keyCode]();
  }
  if (mapping["noctrl" + e.keyCode]) {
    e.preventDefault();
    mapping["noctrl" + e.keyCode]();
  }
});

pretty simple, just change your mapping to always include the value of e.ctrlKey ... mapping[e.ctrlKey +":"+e.keycode] — technosaurus
– technosaurus, Commented Nov 10, 2013 at 19:49

Pointy · Accepted Answer · 2013-11-10 20:01:54Z

2

You could save the result of the logic in a variable, and then check separately.

var fn = e.ctrlKey && mapping[e.keyCode] || mapping['noctrl' + e.keyCode];
if (fn) {
  e.preventDefault();
  fn();
}

The assignment is equivalent to:

var fn;
if (e.ctrlKey && mapping[e.keyCode])
  fn = mapping[e.keyCode];
else
  fn = mapping['noctrl' + e.keyCode];

Or, a little shorter:

var fn = e.ctrlKey ? mapping[e.keyCode] : mapping['noctrl' + e.keyCode];

Or, while we're having fun with variations, this is a little more DRY:

var fn = mapping[(e.ctrlKey ? "" : "noctrl") + e.keyCode];

Note that the second if test could also be rewritten:

fn && (e.preventDefault(), fn());

That's a little too terse for me however.

edited Nov 10, 2013 at 20:01

answered Nov 10, 2013 at 19:32

Pointy

415k62 gold badges600 silver badges631 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

Bali Balo Over a year ago

It won't work, as fn would not contain a function anymore but a boolean.

Pointy Over a year ago

@BaliBalo I'm afraid it will work, as the JavaScript && and || operators don't work the way you think they do!

Pointy Over a year ago

@BaliBalo try this: var f = true && function() { alert("Hi!"); }; f();

Bali Balo Over a year ago

you're right, sorry. I didn't check it and forgot this behaviour. It would be clearer with var fn = e.ctrlKey ? mapping[e.keyCode] : mapping['noctrl' + e.keyCode]; though.

Pointy Over a year ago

@user2965293 you'd need the second if statement, as in the code in my answer.

Collectives™ on Stack Overflow

simplifying this block of javascript code (if)

1 Answer 1

5 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

5 Comments

Your Answer

Sign up or log in

Post as a guest

Related