3
\$\begingroup\$

I'm developing a Poker app. It is almost done, and I'm looking for improvement. One of the things I wonder is whether I should change how I perform iterations. I currently iterate by using tail-recursive calls. However, a friend of mine suggested that I use a while loop instead because loops don't require stack space.

private async Task Turns()
    {
        _turns = ReturnTurns();
        GC.KeepAlive(Updates);
        if (!Player.FoldTurn && Player.Chips > 0)
        {
            if (Player.Turn)
            {
                SetPlayerStuff(true);
                Call -= Player.PreviousCall;
                _up = int.MaxValue;
                _turnCount++;
                Bot1.Turn = true;
                _restart = true;
            }
        }
        if (!Player.Turn)
        {
            await Flip(0);
        }
        if (Player.FoldTurn || !Player.Turn || Player.Chips <= 0)
        {
            Call = TempCall;
            if (StatusLabels[Player.EnumCasted].Contains(RepetitiveVariables.Fold))
            {
                Bot1.Turn = true;
            }
            SetPlayerStuff(false);
            Bot1 = (Bot)await RotateTurns(Bot1, Bot1.EnumCasted);
            Bot2 = (Bot)await RotateTurns(Bot2, Bot2.EnumCasted);
            Bot3 = (Bot)await RotateTurns(Bot3, Bot3.EnumCasted);
            Bot4 = (Bot)await RotateTurns(Bot4, Bot4.EnumCasted);
            Bot5 = (Bot)await RotateTurns(Bot5, Bot5.EnumCasted);
            _restart = false;
        }
        if (!_restart)
        {
            await Turns();
        }
    }

That's how I think it should look like with a while loop:

    private async Task Turns()
    {
        while (true)
        {
            _turns = ReturnTurns();
            GC.KeepAlive(Updates);
            if (!Player.FoldTurn && Player.Chips > 0)
            {
                if (Player.Turn)
                {
                    SetPlayerStuff(true);
                    Call -= Player.PreviousCall;
                    _up = int.MaxValue;
                    _turnCount++;
                    Bot1.Turn = true;
                    _restart = true;
                }
            }
            if (!Player.Turn)
            {
                await Flip(0);
            }
            if (Player.FoldTurn || !Player.Turn || Player.Chips <= 0)
            {
                Call = TempCall;
                if (StatusLabels[Player.EnumCasted].Contains(RepetitiveVariables.Fold))
                {
                    Bot1.Turn = true;
                }
                SetPlayerStuff(false);
                Bot1 = (Bot) await RotateTurns(Bot1, Bot1.EnumCasted);
                Bot2 = (Bot) await RotateTurns(Bot2, Bot2.EnumCasted);
                Bot3 = (Bot) await RotateTurns(Bot3, Bot3.EnumCasted);
                Bot4 = (Bot) await RotateTurns(Bot4, Bot4.EnumCasted);
                Bot5 = (Bot) await RotateTurns(Bot5, Bot5.EnumCasted);
                _restart = false;
            }
            if (!_restart)
            {
                continue;
            }
            break;
        }
    }
\$\endgroup\$
  • 2
    \$\begingroup\$ You can't have tail recursion with async it's all rewritten to use a state machine at compile time. \$\endgroup\$ – RobH Feb 24 '16 at 12:53
  • \$\begingroup\$ Can we see more context for this (the class it's in, some of the other methods/properties being referenced), or would that make the question too huge? There's some weird-looking stuff in there, but without seeing more it's hard to know why \$\endgroup\$ – Ben Aaronson Feb 24 '16 at 12:57
  • \$\begingroup\$ C# doesn't do tail-recursion anyways. You wrote a recursive method, not a tail-recursive method. stackoverflow.com/questions/491376/… \$\endgroup\$ – user2023861 Feb 24 '16 at 16:42
5
\$\begingroup\$
GC.KeepAlive(Updates);

Do you really need to do this? Objects are not collected as long there is a valid reference to them meaning that Updates should always be avaiable if you are not replacing the reference. If you are replacing the reference it means that you are not keeping you object alive long enough for the current game which sounds leaky.

Bot1 = (Bot)RotateTurns(Bot1, Bot1.EnumCasted);

RotateTurns should be an instance method on Bot and should not be present on the current scope. And once again you are replacing the object reference. The bots should be alive at least as long as a game instance.

It would be better to keep a list of all bots so you could iterate on them.

foreach(var bot in _bots){
    await bot.RotateTurns();
}
if (!_restart)
{
    continue;
}
break;

The !_restart condition could be placed on the while. Also consider to keep this a method variable instead of a field.

while(!_restart)
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.