CODEWITHSUNDEEP
c#iteration
Simon Verbeke
Simon Verbeke

Reputation: 3005

stop iterating after returning a true?

(couldn't think of a better title, feel free to edit it to a title that describes the question better)

I have the following method:

bool CalculateNewState(int adjacent, bool currentState)
    {
        if (currentState == true)
        {
            foreach (int n in liveRule)
            {
                if (adjacent == n)
                {
                    return true;
                }               
            }
            return false;
        }
        else
        {
            foreach (int n in becomeAliveRule)
            {
                if (adjacent == n)
                {
                    return true;
                }               
            }
            return false;
        }
    }

This is for a game of life clone. What I want to implement is that a user can make his own rules.

The bool currentState tells the method whether the cell is alive or not. the int adjacent tells the method how many alive neighbors the cell has.

What I want to achieve with this is that when the user says: 2,3 and 5 neighbors keep the cell alive. That it will iterate through an array (liveRule) that holds 2,3 and 5. when any match occurs it should return true, else false.

What happens here is that, after returning a true, it keeps iterating and will eventually return whether the last element in liveRule matched.

What do I need to do, to stop iterating after a match has occurred?

It is of course possible I'm taking the wrong approach to this problem. I started from the suggestions here.

(tried to describe it to the best of my abilities, but it still seems quite unclear)

This is C# in Unity3D.

Upvotes: 0

Views: 459

Answers (5)

Zensar
Zensar

Reputation: 817

Well you can always use a "break" statement to terminate the loop. Do something like:

bool CalculateNewState(int adjacent, bool currentState)
{
    if(currentState)
    {
        return IsStateMatch(adjacent, liveRule);
    }
    else
    {
        return IsStateMatch(adjacent, becomeAliveRule);
    }
}

bool IsStateMatch(int adjacent, int[] rules)
{
    bool finalState = false;

    if(rules != null)
    {
        for(int i = 0; i < rules.length; i++)
        {
            if(adjacent == rules[i])
            {
                finalState = true;
                break;
            }
        }
    }

    return finalState;
}

I broke down the methods a little more, just for readability, but I think this is the basic idea. Now, I do agree with the other posters about what could be happening. If your loop is continuing after a break / return statement, then you most likely have buggy code elsewhere incorrectly calling the method.

Upvotes: 1

D&#39;Arcy Rittich
D&#39;Arcy Rittich

Reputation: 171411

Your return statements will exit the CalculateNewState method immediately. If you find that the iteration is continuing, either you are not hitting the return statements (adjacent == n is never true), or possibly CalculateNewState is being called repeatedly from elsewhere in your code.

You can probably rewrite it much more simply to something like:

if (currentState)
    return liveRule.Contains(adjacent);
return becomeAliveRule.Contains(adjacent);

Upvotes: 1

Eric Lippert
Eric Lippert

Reputation: 660088

The code you've implemented says "if adjacent is unequal to any of 2, 3 or 5, then return". Obviously adjacent cannot be equal to all of them!

Start over. Rename your methods so that they are more clear. Booleans should answer a true/false question, so choose names that ask a question:

bool IsCellAlive(int adjacentCount, bool isAlive)
{
    if (isAlive)
        return liveRule.Contains(adjacentCount);
    else
        return deadRule.Contains(adjacentCount);
}

"Contains" is slower than a foreach loop, so this might cause a performance problem. Don't worry about it for now; you haven't even got the code correct yet. Write the code so that it is obviously correct, and then use a profiler to find the slow spot if it is not fast enough.

Remember: make it correct, then make it clear, then make it fast.

Upvotes: 6

Kevin Le - Khnle
Kevin Le - Khnle

Reputation: 10857

Can you use a for loop instead of foreach with an additional variable?

bool CalculateNewState(int adjacent, bool currentState)
{
    if (currentState == true)
    {
        bool match = false;
        for(int n = 0; n < liveRule.length && !match; n++)
        {
            if (adjacent != n)
            {
                match = true;
            }               
        }
        return match;
    }
    else
    {
        bool match = false;
        for(int n = 0; n < becomeAliveRule.length && !match; n++)
        {
            if (adjacent != n)
            {
                match = true;
            }               
        }
        return match;
    }
}

Upvotes: 0

Tevo D
Tevo D

Reputation: 3381

It looks like your equality test is the culprit... shouldn't you be testing adjacent == n instead of adjacent != n? That way it will return true for the matches and only return false if no match.

The iterator will NOT continue after a return exits the loop.

Upvotes: 0

Related Questions