0

I'm working on my little game project as a way to learn and practice C# and I've encountered a design problem. Let's suppose we have following set of classes:

interface IGameState
{
    //Updates the state and returns next active state
    //(Probably itself or a new one)
    IGameState Tick();
}
class Game
{
    public Game(IGameState initialState)
    {
        activeState = initialState;
    }
    public void Tick()
    {
        activeState = activeState.Tick();
    }
    IGameState activeState;
}

Game is basically a state machine for GameStates. We could have MainMenuState,LoadingState or SinglePlayingState. But adding MultiplayerState (which would represent playing a multiplayer game) requires a socket to connect to the server:

class MultiplayerState : IGameState, IDisposable
{
    public IGameState Tick()
    {
        //Game logic...
        //Communicate with the server using the Socket
        //Game logic...
        //Render the game
        return this;//Or something else if the player quits
    }

    public void Dispose()
    {
        server.Dispose();
    }
    //Long-living, cannot be in method-scope
    Socket server;//Or similar network resource
}

Well and here's my problem, I cannot pass it to Game because it doesn't know it should dispose of it and the calling code cannot easily know when the game doesn't need it anymore. This class design is almost exactly what I have implemented so far and I would be fine with adding IDisposable to IGameState but I don't think its a good design choice, after all not all IGameStates have resources. Furthermore this state machine is meant to be dynamic in a sense that any active IGameState can return new state. So Game really doesn't have know which are disposable and which are so it would have to just test-cast everything.

So this got me asking few questions:

  • If I have a class that claims the ownership over an argument of non-sealed type (e.g. initialState in Game's ctor) should I always assume it can be IDisposable? (Probably not)
  • If I have an IDisposable instance should I ever give up its ownership by casting to a base not implementing IDisposable? (Probably no)

I gather from this that IDisposable feels like a quite unique interface with significant lossy(*) semantics - it cares about its own lifetime. That seems in direct conflict with idea of GC itself that offers guaranteed but non-deterministic memory management. I come from C++ background so it really feels like it tries to implement RAII concept but manually with Dispose(destructor) being called as soon as there are 0 references to it. I don't mean this as a rant on C# at all more like am I missing some language feature? Or perhaps C#-specific pattern for this? I know there's using but that's method-scope only. Next there are finalizers which can ensure a call to Dispose but are still nondeterministic, is there anything else? Perhaps automatic reference counting like C++' shared_ptr?

As I've said the above example can be solved (but I don't think it should) by different design but doesn't answer cases where that might not be possible, so please don't focus on it too much. Ideally I would like to see general patterns for solving similar problems.

(*) Sorry, perhaps not a good word. But I mean that a lot of interfaces express a behaviour and that if class implements said interface it just says "Hey, I can also do these things but if you ignore that part of me I still work just fine". Forgetting IDisposable is not lossless. I've found following question which shows that IDisposable spreads by composition or alternatively it could spread through inheritance. That seems correct to me, requires more typing, but OK. Also that's exactly how MultiplayerState got infected. But in my example with Game class it also wants to spread upstream and that doesn't feel right.

Last questions might be if there should even be any lossy interfaces, like if it's the right tool for the job and in that case what is? Or are there other commonly used lossy interfaces that I know should about?

Quimby
  • 17,735
  • 4
  • 35
  • 55

1 Answers1

0

All of your questions are valid discussions; however, when it comes to IDisposable you are in an unknown condition if you pass it to a type, not knowing if that type will dispose of it properly. For this reason, as a rule, the original owner / initializer of the disposable type should always be in charge of disposing.

So in your case, whoever instantiates MultiplayerState is responsible for disposing of it also. If you have to instantiate it, then pass it to the GameState and dispose of it later then the original owner of MultiplayerState should be required to track that somehow and dispose of it properly.

Also, when implementing IDisposable I highly recommend adding disposing to the destructor of the class as well. This is a fail safe, incase the disposable type isn't disposed of properly or properly implemented.

Example:

 public void Dispose()
 {
      server.Dispose();
      GC.SuppressFinalize(this);
 }

 ~MultiplayerState() => Dispose()

I talk about this more here if you're interested.

Michael Puckett II
  • 6,586
  • 5
  • 26
  • 46
  • 1
    Good information about using `IDisposable` - but you should *never* create a finalizer unless your class uses unmanaged resources, and in that case the finalizer should *only* clean up the unmanaged resources. More info [here](https://blogs.msdn.microsoft.com/tom/2008/04/25/understanding-when-to-use-a-finalizer-in-your-net-class/) and [here](https://stackoverflow.com/a/4899622/3225495). – BJ Myers Jul 15 '18 at 19:19
  • @BJMyers I never suggested adding a finalizer just for fun... When you implement ```IDisposable``` it is good practice to implement the finalizer as I've done here. If ```Dispose``` is called then you also call ```GC.SuppressFinalize(this);```, which removes the finalizer from the queue to be finalized during cleanup, implying the cleanup has already been done. .NET classes also do this to protect unmanaged resources from causing memory leaks in the event the object isn't disposed of properly. I highly suggest doing it this way. – Michael Puckett II Jul 15 '18 at 20:54
  • 1
    Finalizers should never touch managed objects, their lifetime cant be guaranteed; bad/strange things can happen. Your example does just that. To quote Eric Lippert, writing a proper finalizer is incredibly hard. You've proven that point with your example. There's no unmanaged resources here, there's no need. If there were, the "proper" pattern is to introduce an overload of Dispose with a bool argument, where the false branch *only* touches unmanaged resources, and call *that* from the finalizer. Just adding a finalizer can have -ve impact since it places the object on the finalization queue. – pinkfloydx33 Jul 15 '18 at 23:50
  • @MichaelPuckettII I vehemently disagree. It is not good practice to routinely implement a finalizer whenever `IDisposable` is used. The articles I linked discuss that at length. – BJ Myers Jul 16 '18 at 01:26
  • No offense but I believe you need a better understanding of garbage collection, disposable patterns, and finalizers in order to make that claim. The example above is just an example and it is indeed the proper way to use a finalizer. – Michael Puckett II Jul 16 '18 at 03:43
  • Thank you for you answer. That seem like a good rule to have, unfortunately that means that you never give up the ownership and the creator must outlive the `IDisposable` object (or create something that does). Furthermore you have to know when to dispose it and to know that `Game` would have to signal it. But at that moment the `IDisposable` already leaked into the `Game` and it might as well do the disposing. I'll just make `IGamestate` Disposable for now, hopefully it won't add so much typing so the solution might be good enough. – Quimby Jul 16 '18 at 11:17
  • 1
    @Quimby The good thing about software development is we all get to be anarchist's from time to time and there's no definite way that works for everyone or every platform. We all have design preferences and teams have unique guidelines that we all need to abide by. Just a though, if the creator of the disposable type isn't outliving the type it's passed to (the lifetime of the application) then it probably shouldn't pass instantiated disposable types out. It kind of works in the same hand as the rule but to each their own and happy programming :) – Michael Puckett II Jul 16 '18 at 16:57