Restart a foreach loop in C#?
Asked Answered
S

5

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..

Subtotal answered 1/1, 2011 at 12:53 Comment(2)
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?Hammack
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...Subtotal
H
68

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);
Holds answered 1/1, 2011 at 12:54 Comment(5)
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.Lattimer
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.Redpencil
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.Embank
@Embank gotophobists tend to think break is okay for some reason.Resemblance
@Resemblance 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.Stephenson
E
13

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>

Embank answered 27/5, 2014 at 7:3 Comment(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.Stephenson
V
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 beginning It is like setting the counter of a for loop to 0 again

Action a;
for(var index = 0; index < Constraints.Count; index++)
{
   if(!Constraints[index].Allows(a))
   {
      a.Change();
      index = -1; // restart
   }
}
Viperine answered 1/1, 2011 at 13:4 Comment(12)
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.Holds
+1 This is the first solution that came to mind -- Why not use a for loop? Why create a bastardized foreach solution.Puffin
@Chuck: Perhaps because not all collections are accessible by index?Holds
@Mehrdad that's possible, but the for loop is where I would start.Puffin
@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.Holds
@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.Puffin
@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.Holds
@Mehrdad I have to admit out of all the solutions posted here the goto solution is the most concise.Puffin
@ 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.Viperine
@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.Holds
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.. :)Subtotal
+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.Retouch
H
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;
}
Hammack answered 1/1, 2011 at 13:4 Comment(6)
+1. Refactoring is usually a good solution in this kind of cases if it's feasible and doesn't make things more complicated.Holds
@John - it will work for IEnumerable as well if you change IList to IEnumerableHammack
I wasn't the downvoter, but you should change IList to IEnumerable.Holds
I did explain. The question did not restrict itself to IList<T>Dalrymple
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.Embank
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.Embank
C
0
for (var en = Constrains.GetEnumerator(); en.MoveNext(); )
{
    var c = en.Current;
    if (!c.Allows(a))
    {
        a.Change();
        en = Constrains.GetEnumerator();
    }
}
Cicala answered 1/1, 2011 at 13:9 Comment(2)
You'll want to Dispose the original first.Dalrymple
@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!Holds

© 2022 - 2024 — McMap. All rights reserved.