5

I noticed that forEach and for in to produce different behavior. I have a list of RegExp and want to run hasMatch on each one. When iterating through the list using forEach, hasMatch never returns true. However, if I use for in, hasMatch returns true.

Sample code:

class Foo {
  final str = "Hello";
  final regexes = [new RegExp(r"(\w+)")];

  String a() {
    regexes.forEach((RegExp reg) {
      if (reg.hasMatch(str)) {
        return 'match';
      }
    });
    return 'no match';
  }

  String b() {
    for (RegExp reg in regexes) {
      if (reg.hasMatch(str)) {
        return 'match';
      }
    }
    return 'no match';
  }
}

void main() {
  Foo foo = new Foo();
  print(foo.a()); // prints "no match"
  print(foo.b()); // prints "match"
}

(DartPad with the above sample code)

The only difference between the methods a and b is that a uses forEach and b uses for in, yet they produce different results. Why is this?

jamesdlin
  • 81,374
  • 13
  • 159
  • 204
kjmj
  • 412
  • 5
  • 19
  • 1
    Code inside `forEach` is executed asynchronously. Therefore `a` returns with 'no match' before any regex is matched. – Hlib Babii Dec 22 '20 at 23:38
  • 2
    @HlibBabii No, that's wrong. `forEach` is *not* asynchronous and *must* not be used with asynchronous operations. The problem here is that the `return` statement returns from different functions. – jamesdlin Dec 23 '20 at 06:07
  • Another alternative: `regexes.any((reg) => reg.hasMatch(str)) ? "match" : "no match"` – lrn Dec 23 '20 at 14:06

2 Answers2

22

Although there is a prefer_foreach lint, that recommendation is specifically for cases where you can use it with a tear-off (a reference to an existing function). Effective Dart recommends against using Iterable.forEach with anything else, and there is a corresponding avoid_function_literals_in_foreach_calls lint to enforce it.

Except for those simple cases where the callback is a tear-off, Iterable.forEach is not any simpler than using a basic and more general for loop. There are more pitfalls using Iterable.forEach, and this is one of them.

  • Iterable.forEach is a function that takes a callback as an argument. Iterable.forEach is not a control structure, and the callback is an ordinary function. You therefore cannot use break to stop iterating early or use continue to skip to the next iteration.

  • A return statement in the callback returns from the callback, and the return value is ignored. The caller of Iterable.forEach will never receive the returned value and will never have an opportunity to propagate it. For example, in:

    bool f(List<int> list) {
      for (var i in list) {
        if (i == 42) {
          return true;
        }
      }
      return false;
    }
    

    the return true statement returns from the function f and stops iteration. In contrast, with forEach:

    bool g(List<int> list) {
      list.forEach((i) {
        if (i == 42) {
          return true;
        }
      });
      return false;
    }
    

    the return true statement returns from only the callback. The function g will not return until it completes all iterations and reaches the return false statement at the end. This perhaps is clearer as:

    bool callback(int i) {
      if (i == 42) {
        return true;
      }
    }
    
    bool g(List<int> list) {
      list.forEach(callback);
      return false;
    }
    

    which makes it more obvious that:

    1. There is no way for callback to cause g to return true.
    2. callback does not return a value along all paths.

    (That's the problem you encountered.)

  • Iterable.forEach must not be used with asynchronous callbacks. Because any value returned by the callback is ignored, asynchronous callbacks can never be waited upon.

I should also point out that if you enable Dart's new null-safety features, which enable stricter type-checking, your forEach code will generate an error because it returns a value in a callback that is expected to have a void return value.

A notable case where Iterable.forEach can be simpler than a regular for loop is if the object you're iterating over might be null:

List<int>? nullableList;
nullableList?.forEach((e) => ...);

whereas a regular for loop would require an additional if check or doing:

List<int>? nullableList;
for (var e in nullableList ?? []) {
  ...
}

(In JavaScript, for-in has unintuitive pitfalls, so Array.forEach often is recommended instead. Perhaps that's why a lot of people seem to be conditioned to use a .forEach method over a built-in language construct. However, Dart does not share those pitfalls with JavaScript.)

jamesdlin
  • 81,374
  • 13
  • 159
  • 204
  • > "Except for simple, specialized cases where the callback is a tearoff (a reference to an existing function), I don't think that it's any simpler than using a basic and more general for loop." Those are exactly the cases where the lint recommends using `forEach`. (Also, it is just a lint, it's entirely optional whether you want to use it). – lrn Dec 23 '20 at 14:08
  • @lrn Ah, I misunderstood the `prefer_forEach` lint. I also remembered that there is an `avoid_function_literals_in_foreach_calls` lint. – jamesdlin Feb 13 '21 at 22:40
0

jamesdin! Everything you have shared about the limitations of forEach is correct however there's one part where you are wrong. In the code snippet showing the example of how you the return value from forEach is ignored, you have return true; inside the callback function for forEach which is not allowed as the callback has a return type of void and returning any other value from the callback is not allowed.

Although you have mentioned that returning a value from within the callback will result in an error, I'm just pointing at the code snippet.

Here's the signature for forEach

Also, some more pitfalls of forEach are:

  • One can't use break or continue statements.
  • One can't get access to the index of the item as opposed to using the regular for loop