4
\$\begingroup\$

I was turned down for a job after a coding take-home test, and some of the feedback I received was:

would have preferred to see more functional programming instead of while loops - and the while loops that were implemented weren't ideal.

Here is the while loop (there was only one) in question:

getDetails (id) {

    return this.$http.get(this.baseUrl + (this.detailPath.replace('{ID}', id)))
    .then((json) => {

        let i = 0;
        let result = [];
        while (i < json.data.length && result.length < 4) {

            if (json.data[i].profile_path) {

                result.push(json.data[i]);

            }
            i++;

        }
        return result;

    });

}

I actually considered using a reducer function instead of the while loop, but there is no way to break out of the reducer once the result length condition (i.e. result.length < 4) is no longer true. So that would mean that very large datasets would increase computation time. I created a jsperf to test this theory.

So, my question is this: am I missing something here? Is there a better, more efficient way to do this that I am overlooking?

\$\endgroup\$
4
\$\begingroup\$

So that would mean that very large datasets would increase computation time.

You should ask if very large datasets are possible. Because if they're not, clean code takes precedence over what would be a premature optimization. All the code inside your then block can be replaced with a single line:

json.data.filter(x => x.profile_path).slice(0, 4)

If performance is truly an issue, your basic thinking is correct, but I'd just use a for loop:

let result = [];
let data = json.data
for (var i = 0; i < data.length; i++) {
  if (data[i].profile_path) result.push(data[i])
  if (result.length == 4) return result
}
// ask interviewer what the behavior should be for incomplete results...

Note that returning inside a for loop will exit the function immediately.

Fwiw, it sounds like the problem you faced was mostly about communication with the interviewer. Next time, politely probe them about what they're looking for, what their assumptions are about the problem, and so on.

\$\endgroup\$
  • \$\begingroup\$ thanks for the answer! Yes, I was able to confirm that datasets could potentially could be large - they were cast/crew members of films - by looking at the data results from the API I was given. As far as the while vs for loop, I did the jsperf and while was just barely more efficient. But yea, I just really wanted to make sure I wasn't overlooking something huge. \$\endgroup\$ – Neil S Oct 3 '17 at 20:54
  • \$\begingroup\$ Sure, np. That said, you really shouldn't be doing perf tests on while vs for loops unless it's for fun. Eg, if an interviewee mentioned that to me, I wouldn't see it as a sign of diligence (although being aware of performance in general is good), I'd see it as a sign of missing the forest for the trees. Or that they didn't understand the difference between micro-optimizations which are almost never needed and big-O thinking, which is important. In this case, the filter solution vs the for/while is O(n) vs O(1). That matters. But optimizations beyond that don't. \$\endgroup\$ – Jonah Oct 3 '17 at 21:03
  • \$\begingroup\$ I have to disagree a bit, as any optimization when dealing with websites absolutely do matter. Thinking that they don't matter is ignoring user experience. But I get your point, and if I knew that the dataset was fixed size I would have absolutely used a O(n) solution. \$\endgroup\$ – Neil S Oct 3 '17 at 21:17
  • 1
    \$\begingroup\$ I need to keep pushing back, bc the difference between a for and while loop won't be detectable by a user. The shortest perceivable delay is about 100ms. I'd guess at least an order of magnitude difference between that and for/while deltas. It's important to recognize these ballparks and know what you shouldn't be thinking about. Saying "it's always better to be faster" is actually a dangerous and costly attitude. You really need to know what you should spend time on. \$\endgroup\$ – Jonah Oct 3 '17 at 21:26
  • \$\begingroup\$ Also, to be clear, it's fine to a use while here -- I just thought the for was slightly cleaner. But it's not fine to think the performance difference is a consideration you should be making :) \$\endgroup\$ – Jonah Oct 3 '17 at 21:29
2
\$\begingroup\$

A functional approach to breaking out of a loop is to use Array.some(). It would be different than using .reduce() because the return value wouldn't be the resulting array, so the array would still need to be declared before the call to .some(). Then return a truthy value (i.e. when result.length > 3) to break out of the loop. This means it isn't a one-liner, but still a functional approach.

With functional approaches, there is often going to be performance loss due to the fact that a function is being called for each item in the list. Here is a modified jsperf. Also, I would recommend going through these functional exercises if you haven't already.

getPromise().then((json) => {
  let result = [];

  json.data.some((item) => {
    if (item.profile_path) {
      result.push(item);
    }
    return result.length > 3; //break out of the loop once 4 results are found
  });
  console.log(' result: ', result);
});

//simulate getting JSON data
function getPromise() {
  return new Promise((resolve, reject) => {
    window.setTimeout(() => resolve(getJson()), 1200)
  });
}
function getJson() {
  return {
    data: Array(20).fill(0).map((value, index) =>{
      let item = {
        'id': 'id' + index
      };
      if (index % 3 == 0) {
        item.profile_path = '/path' + index;
      }
      return item;
    })
  };
}

\$\endgroup\$
  • \$\begingroup\$ Thanks for your answer! I have to admit, I didn't think about using some in that way. But looking at the jsperf results from the modified one you created, it still looks like while blows it away, performance-wise. \$\endgroup\$ – Neil S Oct 3 '17 at 21:08
  • \$\begingroup\$ Yeah; I have also used .some() as a substitute for .find(), since some browsers have no support for it (e.g. IE) and I didn't care to add a polyfill, as well as breaking out of a loop after splicing \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Oct 3 '17 at 21:11

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.