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

While loop uses extra variables - can this be cleaned up?

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

Problem

A piece of my program allows the looping and incrementing of a targeted entity to be used in a while loop somewhere else. Here is a sample of the while loop:

while (state.World.Looping(reqComponents))
{
    TimerComponent timer = state.World.GetComponent(TimerComponent.Type);

    if (timer.IsActive)
    {
        timer.TimeLeft -= gameTime.ElapsedGameTime.TotalSeconds;

        if (timer.IsFinished)
        {
            timer.IsActive = false;
        }
    }
}


Here is the actual loop function, which increments the current entity (which is grabbed by state.World.GetComponent():

public bool Looping(ComponentType reqComponents)
{
    if (!isLooping)
    {
        isLooping = true;
        CurrentEntity = 0;
    }
    else if (stopLooping)
    {
        isLooping = false;
        stopLooping = false;

        return false;
    }
    else if (++CurrentEntity == EntityCount - 1)
    {
        stopLooping = true;
    }

    if (!HasComponents(reqComponents))
    {
        return Looping(reqComponents);
    }

    return true;
}


The very top part of the Looping() feels really ugly to me. Is there a better way to do this?

Solution

Basically, this code controls everything except the body of the loop: initialization, increment, and destruction. This suggests that the actual while should be inside State.World.Looping, and it should take another parameter, an Action or equivalent delegate, which the caller should pass in. For instance, the call would be rewritten:

state.World.Looping(reqComponents, () =>
{
    TimerComponent timer = state.World.GetComponent(TimerComponent.Type);

    if (timer.IsActive)
    {
        timer.TimeLeft -= gameTime.ElapsedGameTime.TotalSeconds;

        if (timer.IsFinished)
        {
            timer.IsActive = false;
        }
    }
});


and Looping itself could be modified to:

public void Looping(ComponentType reqComponents, Action loopBody)
{
    if (!HasComponents(reqComponents))
    {
       return;
    }
    CurrentEntity = 0;

    do
    {
       loopBody();
    } while (++CurrentEntity != EntityCount - 1);
}


I think I got the number of iterations right but I'm not completely sure.

Code Snippets

state.World.Looping(reqComponents, () =>
{
    TimerComponent timer = state.World.GetComponent(TimerComponent.Type);

    if (timer.IsActive)
    {
        timer.TimeLeft -= gameTime.ElapsedGameTime.TotalSeconds;

        if (timer.IsFinished)
        {
            timer.IsActive = false;
        }
    }
});
public void Looping(ComponentType reqComponents, Action loopBody)
{
    if (!HasComponents(reqComponents))
    {
       return;
    }
    CurrentEntity = 0;

    do
    {
       loopBody();
    } while (++CurrentEntity != EntityCount - 1);
}

Context

StackExchange Code Review Q#59600, answer score: 3

Revisions (0)

No revisions yet.