Made FluidityRepository disposable #50
Conversation
|
Nice addition. IFluidityRepository is obsolete because I don't want people using it directly, however internally we do treat everything as IFluidityRepository (Abstract repo and the default repo are both IFluidityRepository, but the default repo doesn't implement the abstract one, hence the need for the intetrface). On the face of it, this seems fine to me, but given the info above, maybe we make IFluidityRepository IDisposable? |
|
Oops, just noticed you actually have. Though that should mean you shouldn't need to implment IDispoable at the concrete class level as it's implied by IFluidityRepository |
| @@ -20,7 +20,7 @@ namespace Fluidity.Data | |||
| /// <typeparam name="TEntity">The type of the entity.</typeparam> | |||
| /// <typeparam name="TId">The type of the identifier.</typeparam> | |||
| /// <seealso cref="Fluidity.Data.IFluidityRepository" /> | |||
| public abstract class FluidityRepository<TEntity, TId> : IFluidityRepository | |||
| public abstract class FluidityRepository<TEntity, TId> : IFluidityRepository, IDisposable | |||
There was a problem hiding this comment.
Is IDisposable necesarry here as IFluidityRepository implement IDisposable anyway?
There was a problem hiding this comment.
It's not. It was only the ObsoleteAttribute on the interface that caused me to add it here, for fear that if the interface was removed without IDisposable being moved over to the abstract class. It sounds like this is unnecessary duplication though so I'll remove it. 2 secs.
There was a problem hiding this comment.
Yea, IFluidityRepository whilst marked as obsolete, won't be removed. It's purely to stop it being used directly.
…itory<TEntity, TId>
|
Not sure if you get alerted to the push I just did for the requested change. |
My FluidityRepository implementation would ideally be IDisposable, as it uses a disposable resource. This PR makes
IFluidityRepositoryandFluidityRepository<TEntity, TId>disposable, though the default repository disposes of nothing. I added it to both only becauseIFluidityRepositoryis marked as obsolete, thought it is clearly widely used so perhaps this is unnecessary.Additionally where an
IFluidityRepositoryobject is instantiated I've disposed of the object after use.This was really quick to achieve. Please know that I take absolutely no offence if you think it isn't a useful inclusion.