-1
\$\begingroup\$

This is real JavaScript/Lodash code used in a project I'm currently working on, but actually this particular question bothers me quite often regardless of the language. Say, I need to generate some data relying on some filtered out piece of other date.

Here are three approaches:

// filtering out, then just map
var activityFields =
 _.map(
  _.pick(options, function(val, key) {return val && userData[key]}),
  function(val, key) {
      return ({name: val, value: userData[key]})
  }
);

// map all, then just filter out what you need
var activityFields =
_.filter(
  _.map(options, function(val, key) {
      return {name: val, value: userData[key]}
  }), function(item) {
      return item.name && item.value;
  }
);

// being more imperative
var activityFields = [];
_.each(options, function(val, key) {
  if (val && userData[key]) {
    activityFields.push({name: val, value: userData[key]})
  }
});

Which one is to better for making an idiom? I caught myself using them interchangeably.

\$\endgroup\$
  • \$\begingroup\$ @Jamal, while I'm interested in idiomatic approach in genera, I've pointed out explicitely that this is a real code I'm working on, not a demo one. \$\endgroup\$ – shabunc Jul 2 '15 at 17:23
  • 2
    \$\begingroup\$ This question is in the reopen queue, allowing the community to make sure of its topicality. \$\endgroup\$ – Jamal Jul 2 '15 at 17:44
  • \$\begingroup\$ In the top two, are the comments reversed? One mentions mapping, then filtering, while the code goes .filter(.map()), and the other mentions filtering, then mapping, while the code goes .map(.pick()) \$\endgroup\$ – user34073 Jul 2 '15 at 18:18
  • 3
    \$\begingroup\$ I voted to leave this question closed in the review queue, because it's a comparative review showing 3 distinct approaches to a problem that isn't presented in full context. Then I came here and voted to reopen, because for now, it's community consensus that comparative reviews are on-topic. However I downvoted it, because I don't agree with this, and it remains a borderline question, but in all fairness I have to lean on the "reopen" side of the fence. It would be much better to include one of these snippets as part of actual code that's using it, and then voicing your concerns about it. \$\endgroup\$ – Mathieu Guindon Jul 2 '15 at 19:02
  • 2
    \$\begingroup\$ Being discussed on meta \$\endgroup\$ – rolfl Jul 2 '15 at 19:21
3
\$\begingroup\$

The mapping step is pointless for something that will be filtered out anyway. Logically, mapping first and filtering later is a waste of CPU cycles. It's also confusing to read, as I tend to give programmers the benefit of the doubt, so when something looks pointless, I have to double check if I'm missing something.

Functional solutions are ideal. They naturally avoid side effects, naturally lead to better encapsulation, and they go hand in hand with immutability and parallelism. Imperative solutions can be useful when the functional approach would be hard to read somehow, or not efficient due to creating too many objects. That's not the case here, the imperative solution had no benefit whatsoever.

In the posted code, among the given alternatives, the first (pick + map) is a clear winner.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.