dotnet-server-sdk
dotnet-server-sdk copied to clipboard
ILdClient No Longer inherents IDIsposable
Describe the bug
In 5.x, ILdClient inherented IDisposable
In 6.x, ILdClient no longer inherents IDisposable
To reproduce Steps to reproduce the behavior.
Expected behavior
If LdClient needs to be disposed, ILdClient should inherent IDisposable
Logs If applicable, add any log output related to your problem.
SDK version 6.3.1
OS/platform Windows 10
Additional context
I have a singleton class that has a ILdClient instance variable. Prior to 6.x, my class' Dispose method called ILdClient.Dispose. With 6.x, that is no longer possible.
Prior to 6.x, my class' Dispose method called ILdClient.Dispose. With 6.x, that is no longer possible.
I'll address your issue more directly in a moment, but to be clear: that's not quite accurate. It's somewhat less convenient to do, but it is still very possible, with a single line of code:
(myClientInstance as IDisposable)?.Dispose()
In that example, if myClientInstance is really an LdClient, and therefore really does implement IDisposable, then the as cast will succeed and Dispose will be called. But if myClientInstance is some other custom implementation of ILdClient that doesn't implement IDisposable, then the as cast will evaluate to null, and because of the ?., it will not try to call Dispose... making this technique always safe to use.
Another, less concise way to accomplish the same thing:
if (myClientInstance is IDisposable d)
d.Dispose();
This pattern is a not-uncommon approach to managing lifecycles of components when you only know their type via a limited interface.
Now, to your original point. I agree that it would have been preferable to keep the inheritance from IDisposable. I don't have a record of any specific intention to remove it, so I think that may have been just an oversight in the 6.0 rewrite.
Unfortunately, this is the kind of thing that can't exactly be fixed immediately in a patch or minor version release, due to our guarantee of backward compatibility within major versions. The compatibility issue may not be immediately obvious, but it's inherent in how interfaces work.
The reason for providing an interface like ILdClient is to support the idea of multiple implementations of that interface— for instance, if you wanted to inject a test fixture instead of a real client. So that means it is a supported use of the API for an application to define its own class that implements ILdClient. Now, say that we change ILdClient so that it extends IDisposable. Now, for any class to implement ILdClient, it must have a Dispose method. Any application code that defined a class with ILdClient, but did not provide a Dispose method—which was previously OK—will no longer compile, so we have made a breaking change. We can only do that in a major version release.
What we could do is define an intermediate interface— called for instance ILdClientDisposable— which does nothing but extend the two interfaces ILdClient and IDisposable. Then, LdClient could implement that interface, and you could use that as the declared type in your code when you want to reference a client instance without its concrete type. That would not be a breaking change.
However, personally my own recommendation in the specific case of lifecycle management is that you not rely on declarations in this way. The "call Dispose if in fact this component's concrete type does implement IDisposable" pattern I showed above is, I think, always the safest way to ensure that things are getting disposed of when appropriate.
That doesn't only apply to cases like this one. It's quite common for classes to implement several interfaces for unrelated areas of functionality, and some of those interfaces may not be defined by the authors of the API (so they don't have the option of adding IDisposable to them), and/or may be abstractions that don't logically have anything to do with whether a thing is disposable or not. If your code happens to only know of this object by one of those interfaces, and you happen to know that it is time to dispose of the object now, then this kind of conditional cast is recommended.
Closing this issue as there is not any recent activity.
Summary: The client itself is disposable, and if using just the interface (ILdClient), then it can be cast to IDisposable as done in this thread.