23

How can I restart a foreach loop in C#??

For example:

Action a;
foreach(Constrain c in Constrains)
{
   if(!c.Allows(a))
   {
      a.Change();
      restart;
   }
}

restart here is like continue or break but it restarts the foreach from the begining It is like setting the counter of a for loop to 0 again..

Is that possible in C#?

Edit:I want to thank both Mehrdad Afshari and Mahesh Velaga for letting me discover a bug (index=0) in my current implementation, that would not have been discovered otherwise..

chiccodoro
  • 14,407
  • 19
  • 87
  • 130
Betamoo
  • 14,964
  • 25
  • 75
  • 109
  • It could be interesting to know where exactly you need to use this kind of restart. You are using a list of mutable objects in some kind of algorithm. Can you share the actual problem you are trying to solve? – Unmesh Kondolikar Jan 01 '11 at 13:09
  • The actual problem: some agents trying to move in an environment. There are barriers in the env. After each agent decides what the next motion action is, the environment checks if the agent will cross the barrier, if yes, the environment allows the agent to choose another action; and here where I need to restart the foreach loop in order to check all the barriers again with the newly selected action.. I hope that makes clear... – Betamoo Jan 01 '11 at 18:20

5 Answers5

64

Use the good old goto:

restart:
foreach(Constrain c in Constrains)
{
   if(!c.Allows(a))
   {
      a.Change();
      goto restart;
   }
}

If you're diagnosed with gotophobia 100% of the time for some reason (which is not a good thing without a reason), you can try using a flag instead:

bool restart;
do {
   restart = false;
   foreach(Constrain c in Constrains)
   {
      if(!c.Allows(a))
      {
         a.Change();
         restart = true;
         break;
      }
   }
} while (restart);
Mehrdad Afshari
  • 414,610
  • 91
  • 852
  • 789
  • 1
    be careful! in dotnet3.5 and beyond (LINQ etc), foreach () can pull in a closure. This solution, or any other that jumps out of the loop, will dispose the closure correctly. But think it through; this is the kind of place that careful design will save lots of debugging. – O. Jones Jan 01 '11 at 13:27
  • why would you want this? I mean what problem does it solve? It'd be nice to see what the original was rather than the contrived code. – default_avatar Jan 14 '14 at 15:45
  • I like the "good old goto" and "gotophobia". However your suggestion for gotophobists is no better at all. `break` is a `goto` just with another name. It only jumps to the wrong place which you have to fix using a flag and an additional do-while loop - much uglier than solution 1. You probably knew that :-) But once you want to suggest a non-goto version, it should be real non-goto. Or you could completely remove that part since it is already covered by Mahesh's answer. – chiccodoro May 27 '14 at 06:53
  • @chiccodoro gotophobists tend to think `break` is okay for some reason. – Wolfzoon Oct 10 '16 at 12:36
  • @Wolfzoon It's true break is sort of a goto, just like throw and return are. The difference is that it's a standardly used goto with a well-defined structure that modern experienced programmers intuitively understand. When reading the raw gotos, you need to take an extra moment to find the label and check for any potential gotchas. And like chiccodoro's answer demonstrates, temptation to use either a goto or add an unwieldy flag is often a hint that some refactoring might help. – relatively_random Jan 21 '21 at 12:59
12

Although a very old thread - none of the answers paid due attention to the semantics of that code:

  • You have a chain of constraints on a
  • If a breaks any of them, try another a and push that through the chain.

That is, a.Change() should be separated from the constraint checking loop, also adhering to the CQS principle:

while (!MeetsConstraints(a))
{
    a.Change();
}

bool MeetsConstraints(Thing a)
{
    return Constraints.All(c => c.Allows(a));
}

No goto, no ugly loops, just simple and clean. </self-back-slapping>

chiccodoro
  • 14,407
  • 19
  • 87
  • 130
  • 1
    This answer clearly demonstrates that statements like "don't be a fanatic, use goto when it's appropriate" usually miss the point: finding a way around the goto almost always makes the code cleaner. Even the often-mentioned use-case of breaking multiple loops can be refactored into a separate method with a return statement. Especially now that C# supports local functions with their closures. – relatively_random Jan 21 '21 at 12:51
9

One way you can do that is using for, as you have already mentioned:

restart here is like continue or break but it restarts the foreach from the begining It is like setting the counter of a for loop to 0 again

Action a;
for(var index = 0; index < Constratins.Count; index++)
{
   if(!Constraints[index].Allows(a))
   {
      a.Change();
      index = -1; // restart
   }
}
Mahesh Velaga
  • 21,633
  • 5
  • 37
  • 59
  • 7
    This is wrong. `index = -1` would work, if the enumerable is index-accessible but even in that case, it makes the code hard to read and makes the intent unclear. This is a perfect instance of making things worse thing as a result of blind gotophobia. At least, with `goto`, the intent is perfectly clear. – Mehrdad Afshari Jan 01 '11 at 13:05
  • +1 This is the first solution that came to mind -- Why not use a for loop? Why create a bastardized foreach solution. – Chuck Conway Jan 01 '11 at 13:08
  • 5
    @Chuck: Perhaps because not all collections are accessible by index? – Mehrdad Afshari Jan 01 '11 at 13:08
  • @Mehrdad that's possible, but the for loop is where I would start. – Chuck Conway Jan 01 '11 at 13:10
  • 1
    @Chuck There are other problems with this approach. It's error-prone (just proven by the original revision of the answer with `index = 0`). Manipulating things with indices is just asking for more problems. Let alone that manipulating a loop index is frowned upon. – Mehrdad Afshari Jan 01 '11 at 13:13
  • @Mehrdad Point taken, but it's still where I would start. There does not seem to be a decisive way of resetting a foreach loop. – Chuck Conway Jan 01 '11 at 13:18
  • 1
    @Chuck: Yes, but I'd argue that you *shouldn't* reset a `for` loop by manipulating the loop index inside the loop. I believe `goto` is *more readable* and *less error prone* than setting the index to -1. – Mehrdad Afshari Jan 01 '11 at 13:20
  • @Mehrdad I have to admit out of all the solutions posted here the goto solution is the most concise. – Chuck Conway Jan 01 '11 at 13:25
  • @ Mehrdad: I agree to all the points mentioned, but this is the first thing that came to mind (inspite of manipulating a loop index being bad). I do like your do-while implementation for this. – Mahesh Velaga Jan 01 '11 at 13:26
  • @Mahesh: I understand. Sorry for sounding too harsh! The reason I argued with your solution a lot was not really about your solution. I found it to be a good case study to demonstrate the fact that not only `goto` is not a monster to be afraid of, but it's actually useful and may lead to clearer code. – Mehrdad Afshari Jan 01 '11 at 13:35
  • I want to thank both Mehrdad Afshari and Mahesh Velaga for letting me discover a bug (index=0) in my current implementation, that would not have been discovered otherwise.. :) – Betamoo Jan 01 '11 at 18:45
  • +1, that was the first (well, the second, after recursion) solution that came to my mind. I think it makes the intent perfectly clear – just as much as the `goto` version, in fact. The caveat about indexable collections still stands, of course. – Konrad Rudolph Jan 02 '11 at 09:20
4
void Main()
{
    IEnumerable<Constrain> cons;
    SomeObject a;

    while(!TryChangeList(cons, a)) { }
}

// the name tryChangeList reveals the intent that the list will be changed
private bool TryChangeList(IEnumerable<Constrain> constrains, SomeObject a)
{
    foreach(var con in constrains)
    {
        if(!c.Allows(a))
        {
            a.Change();
            return false;
        }
    }
    return true;
}
Unmesh Kondolikar
  • 9,256
  • 4
  • 38
  • 51
  • +1. Refactoring is usually a good solution in this kind of cases if it's feasible and doesn't make things more complicated. – Mehrdad Afshari Jan 01 '11 at 13:11
  • @John - it will work for IEnumerable as well if you change IList to IEnumerable – Unmesh Kondolikar Jan 01 '11 at 13:15
  • I wasn't the downvoter, but you should change `IList` to `IEnumerable`. – Mehrdad Afshari Jan 01 '11 at 13:16
  • 1
    I did explain. The question did not restrict itself to `IList` – John Saunders Jan 01 '11 at 13:20
  • IMO the name "TryChangeList" does more confuse than reflect the meaning of the code. First, you don't change a list, `a` is simply a `SomeObject` (or an `Action` in the OP's code). Second, TryChangeList sounds as if the list/action was changed if it was successful. It is the opposite. If the value of `a` does *not* meet the constraints, a new value needs to be tested. – chiccodoro Aug 22 '14 at 10:22
  • 1
    The proper method name would be `CheckConstraintsAndIfNotMetChangeAOnceWithoutCheckingConstraintsAfterwards`. Or you could remove the line `a.Change()` and put it in the `while` body, makeing the method pure constraint-checking: `while (!MeetsConstraints(cons, a)) { a.Change(); }`. Refactoring is a good idea, but do it properly. This way the method still mixes value-changing and constraint-checking logic, and value-changing is now spread across two methods. – chiccodoro Aug 22 '14 at 10:25
0
for (var en = Constrains.GetEnumerator(); en.MoveNext(); )
{
    var c = en.Current;
    if (!c.Allows(a))
    {
        a.Change();
        en = Constrains.GetEnumerator();
    }
}
Eivind T
  • 1,059
  • 9
  • 14
  • 4
    You'll want to Dispose the original first. – John Saunders Jan 01 '11 at 13:12
  • 3
    @John: Indeed. And you have to deal with potential exceptions, ... which eventually leads to a `using` statement, and at that point, you'd realize that you shouldn't have ditched `foreach` in the first place! – Mehrdad Afshari Jan 01 '11 at 13:16