0
\$\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.

200_success
146k22 gold badges190 silver badges478 bronze badges
asked Jul 2, 2015 at 16:58
\$\endgroup\$
6
  • \$\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\$ Commented Jul 2, 2015 at 17:23
  • 2
    \$\begingroup\$ This question is in the reopen queue, allowing the community to make sure of its topicality. \$\endgroup\$ Commented Jul 2, 2015 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\$ Commented Jul 2, 2015 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\$ Commented Jul 2, 2015 at 19:02
  • 2
    \$\begingroup\$ Being discussed on meta \$\endgroup\$ Commented Jul 2, 2015 at 19:21

1 Answer 1

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.

200_success
146k22 gold badges190 silver badges478 bronze badges
answered Jul 3, 2015 at 5:31
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.