3

I want to iterate using forEach and, when the correct element's been located, return the value. I was under impression that a simple return inside the forEach would suffice but learned that the result in the calling statement was undefined.

So I had to declare an output variable scoped to the whole of the method and tick through all the elements, despite the fact that the right one's already been found. (The elements are unique.)

getMenuIndex(url) {
  let output = -1;

  this.menus.forEach(app => {
    app.subs.forEach(sub => {
      console.log(sub.link + " vs " + url);
      if (sub.link === url)
        // return sub.id;
        output = sub.id;
    });
  });

  return output;
}

This is code smell long way. Is there a neater way to pick the ID of the element matching the URL condition?

Estus Flask
  • 206,104
  • 70
  • 425
  • 565
Konrad Viltersten
  • 36,151
  • 76
  • 250
  • 438
  • 1
    [There is no built-in break for forEach](http://stackoverflow.com/questions/2641347/how-to-short-circuit-array-foreach-like-calling-break) – Hodrobond Apr 13 '17 at 17:22
  • Instead of `forEach` you can use `find`. With `Array.find` you can stop the iteration by returning `true`. – Titus Apr 13 '17 at 17:30
  • @Titus True but not applicable in my case. The thing, as shown in the example is a nested creature. In the actual program, the nesting has even more levels of depth. Otherwise, good suggestion, mate. – Konrad Viltersten Apr 13 '17 at 19:34
  • @NinaScholz I think that `typescript` tag is important here because it provides the context for the supposed use - as it was shown in the answer. Generally for..of is slower than forEach, but in transpiled TS ES5 target it is faster. – Estus Flask Apr 13 '17 at 21:26
  • @estus, i dont see any typescript in the question, but anyway. – Nina Scholz Apr 13 '17 at 21:28

2 Answers2

5

Luckily, we have for..of in TypeScript and ES6 for such scenarios:

getMenuIndex(url) {
  for (const app of this.menus) {
    for (const sub of app.subs) {
      console.log(sub.link + " vs " + url);
      if (sub.link === url)
        return sub.id;
    }
  }

  return -1;
}

It is very helpful in the cases where forEach limitations are evident, such as await/yield or return/break.

As the other answers say, there are array methods that may fit the case better. Although for TypeScript ES5 target for..of is transpiled to regular for and thus is the fastest solution per se.

Estus Flask
  • 206,104
  • 70
  • 425
  • 565
  • I prefer the lambda styled expressions because they look to me more sexy. However, I see your point and you're probably right, so I agree with it. – Konrad Viltersten Apr 13 '17 at 19:37
  • Me probably too. With functional approach it would likely be something like `const { id = -1 } = this.menus.reduce(...).find(...) || {}; return id`, but it wouldn't be as effective, and it's a bit harder to read even if a developer is into FP. – Estus Flask Apr 13 '17 at 21:18
1

You could use Array#some and exit with true.

function getMenuIndex(url) {
    let output = -1;
    this.menus.some(app => app.subs.some(sub => {
        if (sub.link === url) {
            output = sub.id;
            return true;
        }
    }));
    return output;
}
Nina Scholz
  • 376,160
  • 25
  • 347
  • 392
  • Yet, you are using `some` in a way **not** semantically compatible with the description you yourself quote. There are some people, with whom I sympathize, that consider this the equivalent of torturing cute little kittens. –  Apr 13 '17 at 18:30
  • @torazaburo, do you mean the wrong quote, or the use of some for not using the result of checking? – Nina Scholz Apr 13 '17 at 18:36