0

Suppose I have a method in client code that expects an interface...

void DoWork(IDataManager data) {
    //use IDataManager interface in the DoWork method
}

And the IDataManager interface looks like this...

public interface IDataManager {
    ITypeAData TypeAData { get; }
    ITypeBData TypeBData { get; }
}

And each IType*Data interface just allows some data to be accessed...

public interface ITypeAData {
    int Value { get; }
}

//similar def for ITypeBData...

Currently the code that is providing the IDataManager to the DoWork client method defines the interfaces kinda like this...

public class DataManager : IDataManager {
    ITypeAData TypeAData { get; private set; }
    ITypeBData TypeBData { get; private set; }

    public DataManager(ITypeAData typeAData, ITypeBData typeBData) {
        TypeAData = typeAData;
        TypeBData = typeBData;
    } 
}

public class TypeAData : ITypeAData {
    private int privateValue;
    public int Value {
        get {
            return privateValue;
        }
        set {
            privateValue = value;
        }
    }
}

//something similar for TypeBData : ITypeBData

And then I do something like this...

DataManager dataManager = new DataManager(new TypeAData(), new TypeBData());
//other setup activities
(dataManager.TypeAData as TypeAData).Value = 0; //value is not relevant

Keep in mind that the code defining the DoWork method is in one assembly, the code that defines the interfaces is in a second, and the code that defines the implementations of the interfaces is in a third.

I think this smells a little because in order to set the value of my dataManager's TypeAData.Value property I need to cast to a concrete implementation of ITypeAData. Is this okay because the code providing the instance of ITypeAData is supposed to know the implementation that it is providing? Or is there some pattern I should be using?

The reason I'm using a setter on the Value properties of TypeAData and TypeBData is because the DoWork method can be called multiple times while the data that TypeAData encapsulates changes. The whole point is to pass contextual information from the place where the DataManager is created into the DoWork method. This contextual information can change over multiple calls to DoWork. Hence a property. I suppose I could always create a new TypeAData and TypeBData every time I create a DataManager and set the Value via the constructor. But for some reason it feels more appropriate to have one TypeAData and one TypeBData and "configure" them before each call to DoWork

  • 1
    Any good reason that you're completely avoiding constructors? – candied_orange Apr 29 at 23:29
0

Firstly, as an aside, you are putting a lot of pointless code in your data classes. The private set is not needed unless you are using an ancient version of the language (removing it works with v6+ of C#, I think):

public class DataManager : IDataManager {
    ITypeAData TypeAData { get; }
    ITypeBData TypeBData { get; }

    DataManager(ITypeAData typeAData, ITypeBData typeBData) {
        TypeAData = typeAData;
        TypeBData = typeBData;
    } 
}

And TypeAData can be hugely simplified:

public class TypeAData : ITypeAData 
{
    public int Value { get; set; }
}

Beyond that, yes casting to the concrete type in the way you are is a definite anti-pattern. And it seems completely unnecessary. Just re-order the code:

//other setup activities
var typeAData = new TypeAData { Value = 0 }; //value is not relevant
var dataManager = new DataManager(typeAData, new TypeBData());

and the need for the cast goes away. And that set could go away too by adding a constructor to TypeAData:

public class TypeAData : ITypeAData 
{
    public int Value { get; }

    public TypeAData(int value) => Value = value;
}

//other setup activities
var typeAData = new TypeAData(0); //value is not relevant
var dataManager = new DataManager(typeAData, new TypeBData());
  • Thanks for the input. Please see my edit in the original post that describes my motivation for a property – Doug Tait Apr 30 at 16:55
  • I would edit my comment above but I meant to say setter* not property* – Doug Tait Apr 30 at 17:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.