Reputation: 2030
Trying to generate a generic queue of filters to be applied to an image (where in the example filter it uses OpenCVSharp.GaussianBlur
, but making it generic so I can plugin any custom filter I create).
I am struggling a bit with the C# generics and intellisense is showing:
cannot convert from 'GaussianBlur' to 'IFilter
Intellisense is recommending changing the following line:
filters.Enqueue(filter);
by casting to the interface
filters.Enqueue((IFilter<IFilterParams>)filter);
however, my question is why is casting required when the concrete class implements the interface and requires it by the generics definitions or am I misunderstanding how to declare the classes using generics.
Current implementation code is as follows:
public class FilterTest
{
private FilterCollection filters = new FilterCollection();
/* ... other irrelevant code ... */
public void ApplyFilters(ref Mat buffer)
{
var filter = new GaussianBlur(new GaussianBlurParams { KernelSize = new Size(6, 6) });
filters.Enqueue((IFilter<IFilterParams>)filter);
filters.Apply(ref buffer);
}
}
. I am extending the Queue<> class for the FilterCollection:
public class FilterCollection : Queue<IFilter<IFilterParams>>
{
public void Apply(ref Mat buffer)
{
while (Count > 0)
Dequeue().Apply(ref buffer);
}
}
and the interfaces for IFilter and IFilterParams are as follows:
public interface IFilter<T> where T : IFilterParams
{
void Apply(ref Mat buffer);
}
public interface IFilterParams { }
And then the sample filter implementation (more or less just a wrapper in this case):
public class GaussianBlurParams : IFilterParams
{
public Size KernelSize = new Size(5, 5);
public double SigmaX = default(double);
public double SigmaY = default(double);
public BorderTypes BorderType = BorderTypes.Default;
}
public class GaussianBlur : IFilter<GaussianBlurParams>
{
private GaussianBlurParams p;
public GaussianBlur(GaussianBlurParams filterParams)
{
this.p = filterParams;
}
public void Apply(ref Mat buffer)
{
Cv2.GaussianBlur(buffer, buffer, p.KernelSize, p.SigmaX, p.SigmaY, p.BorderType);
}
}
So given:
GaussianBlur
implements IFilter<GaussianBlurParams>
IFilter<T> where T : IFilterParams
GaussianBlurParams
implements IFilterParams
Is casting the only way to fix this or is there something wrong with the structure of the generic classes/interfaces as written?
Upvotes: 7
Views: 6612
Reputation: 14007
There are multiple aspects to your code that get entangled here and that altogether make the design that less than optimal. At first glance this might look like an issue of covariance, but on a closer look this is not the case. The two main aspects here are generic constraints and interfaces. To understand what I mean, let's take a look at some of the benefits of these two language elements.
Generic constraints
While generics enable you to use an implementation of a pattern for multiple types in a type-safe way, there is not much you can do to directly manipulate an object of type T
from within the generic class. You can not create an instance, you can not rely on instances being reference or value types (try a comparison with null
to see what that means) and you can not access any other members than the ones defined in System.Object
. That's why you can use generic constraints to allow code within the generic class that can do additional things with objects of type T
, like creating instances (with the new()
constraint) or access additional members (by constraining T
to a certain type and/or one or more interfaces).
Interfaces
Interfaces provide a contractual guarantee that the implementer will have a defined set of members. This guarantee is directed to consumers of the interface, not to its implementers. That means that you don't use interfaces to force its implementer to provide some members that are not of any value to the consumer of the interface.
What this means in your case
The key to your problems is this part of your code:
public interface IFilter<T> where T : IFilterParams
{
void Apply(ref Mat buffer);
}
public interface IFilterParams { }
In particular:
You define the generic constraint where T : IFilterParams
, but IFilterParams
provides no members. This constraint adds no value to your design. You restrict the implementer to a certain T
, but you don't gain anything from it, because you can't do anything with instances of T
that you couldn't do without the constraint.
Going one step further, you don't need the interface to be generic at all. You don't even use T
in the only member that the interface provides. You could do just fine without it as far as the guarantees of the interface are concerned.
Taking a look at the GaussianBlur
implementation of IFilter<T>
, it is clear that you use the GaussianBlurParams
only in the constructor, which is not part of the interface. So you are using the constraint of the interface only to restrict the implementer to use a Params
class that implements IFilterParams
. That is not even a real restriction, because the implementer can use any other parameter class for its initialization. But mainly this violates the principle that the interface provides a guarantee to its consumers, not a restriction its implementers.
Putting this all together, you can simply go for...
public interface IFilter
{
void Apply(ref Mat buffer);
}
...and you have avoided all the problems you are facing.
Even if you would need T
with the constraint where T : IFilterParams
for another consumer of the interface (maybe there is another interface member that you have not added in your example), your FilterCollection
does not need this constraint. So you can still keep a non-generic IFilter
and provide another interface (that might or might not inherit from IFilter
) that provides the additional capabilities.
Upvotes: 4
Reputation: 2030
Ok, thanks to @zzxyz's original comment along with the comments and answer that were added and then shortly after removed, lead me to research more on covariance (which I created by adding the generic IFilterParams in order to avoid covariance), and answers/comments in SO (Contravariance? Covariance? What's wrong with this generic architecture...?) helped me correct my problem and better structure the code.
Now I understand how I was trying to 'add a banana to a bowl(fruit) (fruit being the covariant as it is not just one 'type' of fruit) when I needed to be adding the banana to a bowl(bananas)'. I am understanding but quite poorly summing up one of the answers that were unfortunately deleted.
In researching, I was able to remove the covariance by creating an abstract class with its own generic type for the filterParams and removing the IFilterParams interface altogether, thus all filters must implement the base abstract class, and now no longer cause covariance.
Since I understand it now, but not well enough to explain (above) clearly, the revised code (below) may help explain better.
First, no changes to the FilterTest class were necessary (except removing the casting from the original example which was the point of the question):
public class FilterTest
{
private FilterCollection filters = new FilterCollection();
public void ApplyFilters(ref Mat buffer)
{
var filter = new GaussianBlur(new GaussianBlurParams { KernelSize = new Size(6, 6) });
filters.Enqueue(filter);
filters.Apply(ref buffer);
}
}
Next, adjusted the Queue so it was not covariant (implements one 'type' IFilter) which exposes the needed 'Apply' method.
public class FilterCollection : Queue<IFilter>
{
public void Apply(ref Mat buffer)
{
while (Count > 0)
Dequeue().Apply(ref buffer);
}
}
public interface IFilter
{
void Apply(ref Mat buffer);
}
And finally removed the IFilterParams as they are no longer relevant to the cause. Now the sample filter implementation looks like:
public class GaussianBlur : IFilter
{
private GaussianBlurParams p;
public GaussianBlur(GaussianBlurParams filterParams)
: base(filterParams)
{
}
public override void Apply(ref Mat buffer)
{
Cv2.GaussianBlur(buffer, buffer, p.KernelSize, p.SigmaX, p.SigmaY, p.BorderType);
}
}
public class GaussianBlurParams
{
public Size KernelSize = new Size(5, 5);
public double SigmaX = default(double);
public double SigmaY = default(double);
public BorderTypes BorderType = BorderTypes.Default;
}
Problem fixed, hopefully helps others!
Upvotes: 2