5
\$\begingroup\$

The following is a basic implementation of a loop which is intended to launch a series of games in a loop (generic ones that can be overridden, but that have always have finite durations). The intention would be that a game can broadcast events to clients and would always broadcast its completion. When there are no clients listening it can Shutdown, and restart again when some clients begin listening again

Is it a good way to approach this? Does it matter that I do not await the _gameScheduler.Start(this) call? Am I dealing with the task loop correctly? Is the timer used correctly?

public class Table
{
    private readonly GameScheduler _gameScheduler;
    public Game Game { get; set; }
    public Table()
    {
        _gameScheduler = new GameScheduler();
        Task schedulerTask = _gameScheduler.Start(this);
    }
    public void Close()
    {
        _gameScheduler.Shutdown();
    }
}

public class GameScheduler
{
    private bool _running = true;
    public async Task Start(Table t)
    {
        while (_running)
        {
            await RunGame(t);
        }
    }

    async Task RunGame(Table table)
    {
        var game = new Game();
        table.Game = game;
        table.Game.Start();
        await table.Game.GameCompletionSource.Task;
    }
    public void Shutdown() { _running = false; }
}

public class Game
{
    System.Threading.Timer _countdownTimer;
    private int _timeLeft = 15000;
    private int _countdownInterval = 1000;
    public TaskCompletionSource<bool> GameCompletionSource;
    public void Start()
    {
        _countdownTimer = new System.Threading.Timer(GameCountdown, null, 0, _countdownInterval);
    }
    public void GameCountdown(object state)
    {
        if (_timeLeft <= 0)
        {
            _countdownTimer.Dispose();
            Finish();
        }
        else
        {
            _timeLeft -= _countdownInterval;
        }
    }
    private void Finish()
    {
        GameCompletionSource.SetResult(true);
    }
}
\$\endgroup\$
1
  • \$\begingroup\$ Welcome to Code Review! That looks like an interesting program, I'm curious what our C# experts will have to say about it. Hope you get some good reviews! \$\endgroup\$ – Phrancis Dec 22 '14 at 19:19
2
\$\begingroup\$

I just want to say that I review C# code style a lot on this site, and this code made me very happy for many seconds, until I spotted this:

public async Task Start(Table t)

So close!

t should, of course, have a much more descriptive name here. You've even done that elsewhere, so it is most likely an oversight on your part.

I don't like underscore notation personally as it smells strongly Hungarian, but it's a commonly-accepted system in C# so I don't have a leg to stand on.

\$\endgroup\$
4
  • \$\begingroup\$ Haha! I think that t may have survived from an earlier refactoring from a situation where I was using an anonymous function. Probably not much of an excuse (and I may be making that up!) \$\endgroup\$ – ChrisT Jan 20 '15 at 10:44
  • \$\begingroup\$ On the underscores, interested to know which approach you prefer to differentiate private members, if any at all? Not sure where I picked up the habit but as you say it is certainly quite common in C# \$\endgroup\$ – ChrisT Jan 20 '15 at 10:47
  • 1
    \$\begingroup\$ It tends to come from the older C++ days, if I recall correctly, but is still common in C#. My personal preference is to simply omit underscores and rely on short methods (so I can usually always see the local declarations) to differentiate. However I should again stress that even Microsoft recommend _notation at times. Another recommended alternative is to use this.x when referencing local members. \$\endgroup\$ – Nick Udell Jan 20 '15 at 10:49
  • \$\begingroup\$ Ah yes, makes sense. Well I guess as long as the intention is clear and it's readable I'm happy with any approach. Thanks for the answer and the compliment \$\endgroup\$ – ChrisT Jan 20 '15 at 10:54

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.