ternary operator inside an arrow function inside forEach

Question

I have this little function that is supposed to remove object properties with even values.

function removeEvenValues(obj) {
  Object.keys(obj).forEach(k => ((!isNaN(obj[k])) && (obj[k] % 2 === 0)) ? delete obj[k] : k);
}

In the else {} part of the ternary operator, how do I leave it empty like in an if (true){ doSomething();} kind of construct? Or does it even make sense to use a fat arrow function in this case?

ternary (3 operands) makes no sense if you just want to use 2 operands. — Quotidian, May 08 '17 at 17:07
@Bergi really? (he asked rhetorically) - I could swear this has come up before and I must have looked it up then too. (I almost never use it.) — Pointy, May 08 '17 at 17:08
@Pointy: [Yup](http://www.ecma-international.org/ecma-262/7.0/index.html#sec-delete-operator). :-) — T.J. Crowder, May 08 '17 at 17:09

score 2 · Accepted Answer · edited May 23 '17 at 12:10

2

Yes, you shouldn't use a concise arrow function or a ternary operator here. Well, you could use && instead of the ternary, but you really shouldn't.

You shouldn't even use forEach in ES6 - just use a proper loop:

function removeEvenValues(obj) {
  for (const k of Object.keys(obj))
    if (!isNaN(obj[k]) && obj[k] % 2 === 0)
      delete obj[k];
}

or (as you probably don't care about inherited properties)

function removeEvenValues(obj) {
  for (const k in obj)
    if (!isNaN(obj[k]) && obj[k] % 2 === 0)
      delete obj[k];
}

edited May 23 '17 at 12:10

Community

1
1

answered May 08 '17 at 17:11

Bergi

630,263
148
957
1,375

Probably worth pointing out how `in` differs from `Object.keys` in terms of the properties handled, particularly as `delete` won't delete the inherited ones. – T.J. Crowder May 08 '17 at 17:13
1

Why should you not use forEach? What is the reasoning behind that? – epascarello May 08 '17 at 17:17
@epascarello `forEach` is only good for side effects, and for side effects `for … of` *statements* are the better solution. People shouldn't use `forEach` for looping, they just tend to try [using `break`](http://stackoverflow.com/q/42728174/1048572), [using `yield`](http://stackoverflow.com/q/34104231/1048572) or [using `await`](http://stackoverflow.com/q/37576685/1048572) within it. – Bergi May 08 '17 at 17:24
@epascarello: It's just an opinion, not a generally-accepted rule. Perfectly fine to use `forEach` in ES2015+ if you want to, provided you don't need to do something it doesn't support (such as stopping early -- in that case, use `some`). But as of ES2015+, there's no strong *reason* to use it unless it happens that you already have a function lying around that does what you want, and `for` plays better with some of the newer structures. You no longer need `forEach` for handy scoping of your loop-local vars, so you can avoid creating and calling functions unnecessarily by using a `for`. – T.J. Crowder May 08 '17 at 17:25
@Bergi than they should be using `some` or `every` if they are looking for reason to break out. So basically it is your personal opinion on should not use forEach. ;) – epascarello May 08 '17 at 17:26
@epascarello No, `some`/`every` are for getting a boolean result, not for performing side effects. One should use the right tool that shows the *intent* of the code best, and `forEach` does look functional despite being imperative. But yes, that's just my professional opinion, and while knowing when which hack might be appropriate I just tend to recommend to generally avoid it. – Bergi May 08 '17 at 17:29

score 1 · Answer 2 · answered May 08 '17 at 17:08

1

There is nothing wrong with using a fat arrow, as it is a function expression, but because you are not returning a value, you shouldn't use a ternary operator. You can do:

function removeEvenValues(obj) {
    Object.keys(obj).forEach(k => {if(!isNaN(obj[k]) && (obj[k] % 2 === 0)) {delete obj[k]}});
}

answered May 08 '17 at 17:08

Joe Lissner

2,181
1
15
21

1

Or if you feel a *really strong need* to avoid `if`, `&&` is basically the two-operand conditional operator: `k => !isNaN(obj[k]) && obj[k] % 2 === 0 && delete obj[k]` No, I am not advocating that. :-) – T.J. Crowder May 08 '17 at 17:12
@T.J.Crowder This would also work fine, but not as readable; but it will probably end up looking close to this if you minify it :P – Joe Lissner May 08 '17 at 17:13
@ Joe - Hence my not advocating it. :-) – T.J. Crowder May 08 '17 at 17:13

score 1 · Answer 3 · answered May 08 '17 at 17:12

You could use a logical AND && and remove some superfluous parentheses.

function removeEvenValues(obj) {
    Object.keys(obj).forEach(k => !isNaN(obj[k]) && obj[k] % 2 === 0 && delete obj[k]);
}

var o = { foo: 41, bar: 42 };
removeEvenValues(o);

console.log(o);

trincot · Answer 4 · 2017-05-08T18:00:37.340

As Nina wrote, you can use logical operators instead of the ternary operator. You can also turn the condition around and use ||.

Be aware though what isNaN returns. For instance isNaN([2]) === false! So instead you might want to test for the primitive number type with typeof.

Finally, recently the Object.entries method has found its way in some browsers, and in this case it can be nice to use instead of Object.keys:

function removeEvenValues(o) {
    Object.entries(o).forEach(([k, v]) => typeof v != "number" || v % 2 || delete o[k]);
}

// Sample
var obj = {
    "a": 2,
    "b": "hello",
    "c": [4],
    "d": 5,
    "e": 6
};

removeEvenValues(obj);

console.log(obj);

ternary operator inside an arrow function inside forEach

4 Answers4