Appropriate pattern for injector that sets a property and client that ONLY gets a property

Question

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

Answer 1

Your code is contradictory.

Here, you state that the content of your ITypeAData object is intended as a read-only value:

public interface ITypeAData {
    int Value { get; }
}

Here, you clearly intend to be able to set the value of your ITypeAData content:

(dataManager.TypeAData as TypeAData).Value = 0; 

This is contradictory.

Either it's intended to be immutable (by the consumer who only knows the object by its interface), or it's not.

• If it is a read-only object, then you shouldn't be trying to set its values here.
• If it isn't a read-only object, then your interface needs to be expanded to also have a setter.
• As a third possibility, if the interface should be read-only but the manager should be able to change the values, then the manager needs to know this object by its concrete type, i.e.:

public class DataManager : IDataManager {
    TypeAData TypeAData { get; private set; }
    // and so on...
}

This can also be done without the manager's consumer having access to the setters, while the manager itself does have access:

public class DataManager : IDataManager {

    private TypeAData _typeAData; // set your values by referencing this

    public ITypeAData TypeAData => _typeAData; // expose this to your consumers
}

This way, the manager can access the setters but the consumer of the manager cannot.

Which of these options is correct is a contextual decision, and your vague example code makes it impossible for us to judge which is the appropriate approach here.

---

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?

Based on the generic example code you've provided, I can't think of a case where knowing a type without declaring it as that known type is a good approach. In most if not all cases, this is polymorphism abuse.

That being said, there may be a contextual consideration why it's an acceptable approach here. For example, some deserialization returns object types and expects its consumer to cast it to the desired type. I don't think this is a great approach for a library, but if that's the library you're working with, you haven't really got a choice.

I can't exclude the possibility that you may need to do it the way you're doing it now, simply because I don't know of any contextual considerations. Though I do doubt that it's better to avoid this approach altogether - if it can be avoided in your case.

Answer 2

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());