HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Using an IEnumerator without a foreach loop in order to simulate a state machine

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withoutienumeratororderloopusingstatemachineforeachsimulate

Problem

I am working with an object that needs to run an ExecuteEffect() method. Sometimes, the ExecuteEffect() only needs to run once, while in other cases, the ExecuteEffect() needs to keep running once / frame until some criteria is met (in my example, a real-time delay).

To solve this, I used an IEnumerator:

public override IEnumerator ExecuteEffect()
    {
        if(delay > 0)
        {
            if(executionStart == -1m)
            {
                executionStart = GetRealTime();
                yield return false;
            }
            if(GetRealTime() > executionStart + delay)
            {
                DealDamage();
                yield return true;
            }
            else
            {
                yield return false;
            }
        }
        else
        {
            DealDamage();
            yield return true;
        }
  }


Then, when I'm running ExecuteEffect(), if the IEnumerator returns false, it gets added to a list of objects that is continuously checked until their ExecuteEffect() method returns true.

GenericEffect ge = GetEffect();
IEnumerator temp = ge.ExecuteEffect();
temp.MoveNext();
if(!temp.Current)
{
    //Foo not finished, add to list of active Foo
    activeEffects.Add(ge);
}


and, elsewhere:

activeEffects.RemoveAll( s =>
                         {
                               IEnumerator temp = s.ExecuteEffect();
                               temp.MoveNext();
                               if(temp.Current)
                               {
                                   return true;
                               }
                               else
                               {
                                  return false;
                               }
                          });


This all feels very wordy, especially the temp variable to hold the IEnumerator and the explicit call to GetNext(). And, of course, I have no idea if this is an utter but

Solution

You could make life slightly easier for yourself by using the iterator slightly more properly:

public override IEnumerator ExecuteEffect()
{
    if(delay  executionStart + delay)
    {
        DealDamage();
        yield break;
    }
    if(executionStart == -1m)
    {
        executionStart = GetRealTime();
    }
    yield return false;
}


Then you could actually use MoveNext 'correctly' as it will return false when you get to the end.

To be completely and brutally honest, I'd still rage erase all of this code if I found it in a project I worked on. Why is it an enumerator of booleans? None of it's particularly obvious.

As an aside:

if(temp.Current)
{
    return true;
}
else
{
    return false;
}


is 100% the same as

return temp.Current;

Code Snippets

public override IEnumerator<bool> ExecuteEffect()
{
    if(delay < 0 || GetRealTime() > executionStart + delay)
    {
        DealDamage();
        yield break;
    }
    if(executionStart == -1m)
    {
        executionStart = GetRealTime();
    }
    yield return false;
}
if(temp.Current)
{
    return true;
}
else
{
    return false;
}
return temp.Current;

Context

StackExchange Code Review Q#108028, answer score: 2

Revisions (0)

No revisions yet.