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

Tail-recursive call vs looping for a Poker app

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

Problem

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;

Solution

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)

Code Snippets

GC.KeepAlive(Updates);
Bot1 = (Bot)RotateTurns(Bot1, Bot1.EnumCasted);
foreach(var bot in _bots){
    await bot.RotateTurns();
}
if (!_restart)
{
    continue;
}
break;
while(!_restart)

Context

StackExchange Code Review Q#120963, answer score: 5

Revisions (0)

No revisions yet.