Flyweight and Factory problem with IDisposable

Question

I seem to be mentally stuck in a Flyweight pattern dilemma.

First, let's say I have a disposable type DisposableFiddle and a factory FiddleFactory:

public interface DisposableFiddle : IDisposable
{
    // Implements IDisposable
}

public class FiddleFactory
{
    public DisposableFiddle CreateFiddle(SomethingThatDifferentiatesFiddles s)
    {
        // returns a newly created fiddle.
    }
}

Then, in my opinion, it's quite clear to the client of FiddleFactory that the factory claims no ownership of the created fiddle and that it's the client's responsibility to dispose the fiddle when done with it.

However, let's instead say that I want to share fiddles between clients by using the Flyweight pattern:

public class FiddleFactory
{
    private Dictionary<SomethingThatDifferentiatesFiddles, DisposableFiddle> fiddles = new ...;        

    public DisposableFiddle CreateFiddle(SomethingThatDifferentiatesFiddles s)
    {
        // returns an existing fiddle if a corresponding s is found,
        // or a newly created fiddle, after adding it to the dictionary,
        // if no corresponding s is found.
    }
}

Then I feel morally obliged to make the factory itself disposable, since it creates the fiddles and keeps references to them during all of their lifetime. But that would cause problems to the clients that assumed they owned the fiddles and should therefore dispose them.

Is the problem actually that I call the factory FiddleFactory instead of, say, FiddlePool, and the "creation" method CreateFiddle instead of GetFiddle? Like this:

public class FiddlePool : IDisposable
{
    private Dictionary<SomethingThatDifferentiatesFiddles, DisposableFiddle> fiddles = new ...;        

    public DisposableFiddle GetFiddle(SomethingThatDifferentiatesFiddles s)
    {
        // returns an existing fiddle if a corresponding s is found,
        // or a newly created fiddle, after adding it to the dictionary,
        // if no corresponding s is found.
    }

    // Implements IDisposable
}

Then it's clearer to the client that it'll not own the returned fiddle and it's the pool's responsibility to dispose the fiddles.

Or can this only be readily solved documentation-wise?

Is there a way out of the dilemma? Is there even a dilemma? :-)

Lycia

Answer 1

I can see two ways out of this problem:

• ThreadPool-style: redesign the classes so the FiddlePool provides an interface to do fiddly things. The pool doesn't hand out Fiddle instances because it has a FiddlePool.PlayFiddle method instead. Since the pool controls fiddle lifetimes, it's responsible for disposing of them.

• SqlConnection-style: modify Fiddle's public dispose method so that it really just returns fiddles to the fiddle pool (which the fiddle class encapsulates). Internally, the fiddle pool takes care of really releasing disposable resources.

Dutch

Thanks, the first one follows the Law of Demeter better and the second fits better in my overall design. Hmmm... – Lycia

I'll put my two cents in for the first approach - I've seen one too many people try to write their own SqlConnnection pool! (Not really, but I have had to explain why it isn't necessary.) – Dutch

Answer 2

I agree with your second opinion. The terminology 'Pool' and 'Get' do make things more clear to the consumer. However, it still does not make things clear enough, and documentation should always be added to ensure a full, valid understanding.

Exarchate

My thoughts exactly. In addition, I would probably not call your class DisposableFiddle unless you want the user to Dispose() it... – Selmner

@Reed: Agreed about DisposableFiddle. Proper naming is critical to conveying intent, and is the first line of defense against incorrect usage. – Exarchate

Heh, yes, I just prepended Disposable to the example name to make it easier to follow in my (confused) steps... – Lycia

Answer 3

You should do something more than just documentation and naming methods to tell clients not to call dispose. Really it would be better to have the clients call dispose so make that the usage pattern. Take some direction from our database connection pools are built.

The database pools a bunch of connections which themselves are pool-aware. Calling code creates a connection, opens it, and calls close (dispose) on it. The calling code doesn't even really know if it's pooled or not, it's all handled internally by the connection class. With a pooled connection, calling Open() is ignored if the connection is already open. Close()/Dispose() is called and in the case of a pooled connection this actually returns the connection back to the pool instead of closing it.

You can do the same thing by creating a PooledFiddle class which overrides Dispose and returns the object to the pool. Ideally the client wouldn't even have to know it's a pooled Fiddle.

Mcnulty

"Ideally the client wouldn't even have to know it's a pooled Fiddle." - yes, I agree on that. I think it's an implementation detail that's better off hidden. – Lycia