1

It's already been covered in a few topics that empty interfaces that are used as markers where identification occurs at run time are code smell, but what about when you have something like this:

public interface IPropertyRetriever<out T>
{
    public string Name { get; }

    public T Retrieve();
}

public interface IComparablePropertyRetriever<out T> : IPropertyRetriever<T> where T : IComparable
{
}

In this example, although IComparablePropertyRetriever<T> is an empty interface, it does add a constraint to what T can be. Would something like this also be considered code smell?

EDIT

Ok so here is both how I'm using it and the use case.

I'm building an expression parser that consists of an expression, an operator and a constant, so I ended up with this:

public interface IOperator<out T>
{
    public IPropertyRetriever<T> LeftObjectRetriever { get; }
      
    public T RightObject { get; }

    public bool Compare();
}

T can only ever be one of 4 types: string, bool?, long? and Version

My operators are things like StartsWith, GreaterThan, LowerEquals, Contains...

Clearly some operators such as StartsWith and Contains only work on strings and that's pretty simple to do, I can just do this:

public class BeginsWith : IOperator<string>
{
    public BeginsWith(IPropertyRetriever<string> leftObjectRetriever, string rightObject)
    {
        LeftObjectRetriever = leftObjectRetriever;
        RightObject = rightObject;
    }

    public IPropertyRetriever<string> LeftObjectRetriever { get; }
        
    public string RightObject { get; }
        
    public bool Compare()
    {
        throw new System.NotImplementedException();
    }
}

But others, such as LowerThan work on all other types, except string. And although I guess I could do something like this (which is ultimately what the IComparablePropertyRetriever<T> interface is doing)

public class LowerThan : IOperator<IComparable>
{
}

It still allows me to pass a string to LowerThan since string implements IComparable

By shifting the responsability to the classes implementing IPropertyRetriever (which I can control far more easily) I can instead do something like this:

public class FileVersionRetriever : IComparablePropertyRetriever<Version>
{
}

And implement LowerThan as follows:

public interface IComparableOperator : IOperator<IComparable>
{
    public new IComparablePropertyRetriever<IComparable> LeftObjectRetriever { get; }
}

public class LowerThan : IComparableOperator
{
}

And I guess now that I've explained that the whole question turned into something completely different which is how to implement this better xD

computeka
  • 15
  • 5
  • 2
    What does the constraint achieve if `T` is not being used in the definition? – Johnathan Barclay Sep 30 '20 at 09:39
  • 1
    The smell here is the use of a `property retriever` interface - what is this trying to achieve? As for the linked 10-year old question, things have changed since and *some* pattern matching cases require "marker" interfaces, at least until C# 10 introduces discriminated unions. – Panagiotis Kanavos Sep 30 '20 at 09:43
  • @JohnathanBarclay I don't follow your reasoning... – InBetween Sep 30 '20 at 09:44
  • @InBetween It was more of a question than a statement. I'd be interested to know how this could be useful. – Johnathan Barclay Sep 30 '20 at 09:47
  • 1
    BTW `IComparablePropertyRetriever` in this case isn't a marker. It add some restrictions but omits important information. I suspect the entire design could be simplified a lot. If you want to "inject" some behavior to classes, you could use default interface member implementations for example. If you want to create class that acts as a proxy, whose properties are loaded from a remote system or database, you could put the loading code in the interface's `Retrieve()` for example. This way you'd avoid inheriting from a base abstract class. Loading the *class* on demand though, would be cleaner – Panagiotis Kanavos Sep 30 '20 at 09:51
  • @PanagiotisKanavos mode info added – computeka Sep 30 '20 at 10:50
  • @JohnathanBarclay added more info – computeka Sep 30 '20 at 10:52

0 Answers0