Is it a bad idea to have hide methods and have different method implementations behind different interfaces?

Question

I have a interface which currently extends IDictionary<> (and an implementation which Extends Dictionary<>), but I want to have an implementation of this interface which does not allow entries to be added or removed (I want to allow existing entries to be changed though). I could just take the ReadOnlyCollection approach and throw NotSupportedException, but this feels a bit wrong.

Instead I'd like to break the interfaces up so I had one for the accessor bits and one for the mutator bits. This is all ok, except that to do this I ended up with something like this (most methods removed for brevity):

public interface IAccessor<TKey, TValue>
    {
    TValue this [TKey key] { get; set; }
    }

and then my original interface became:

public interface IAttributeDictionary<TKey, TValue> : IDictionary<TKey, TValue>, IAccessor<TKey, TValue>
    {
    new TValue this [TKey key] { get; set; }
    }

And an implementation class defined as:

public class AttributeDictionary<TKey,TValue>: Dictionary<TKey, TValue>, IAttributeDictionary<TKey, TValue> 

I had to make the indexer new to avoid ambiguities between the indexers in IDictionary and IAccessor. the real issue though is that the behaviour of the setter indexer on a Dictionary is to create a new entry in the dictionary. As I want the IAccessor interface to only allow entries to be modified and not created what should I do in the inmplementation of AttributeDictionary? should I have an explicit implementation of the IAccessor indexer method which first checks the given key is in the dictionary and throws an exception if not, or would having 2 indexers with different behaviour be a bad idea? Or should I ditch the indexer in the IAccessor interface and just have a GetValue and SetValue methods instead and avoid the confusion?

Answer 1

What strikes me as the problem is that you're still trying to implement IDictionary -- I think you shouldn't implement that interface on your AttributeDictionary (because you don't really support the full functionality mandated by the interface). If, however, you have to support it because you need to send instances of AttributeDictionary to methods that take IDictionary and there are no interfaces higher up in the implementation chain of IDictionary you can use, I think the next best thing is to simply implement IDictionary alone and throw in the indexer's setter.

I feel like the approach you're trying now is just going to lead to subtle bugs down the line where you call the wrong indexer without really knowing it, particularly when working with instances of the class via the interfaces themselves.

EDIT: After Sam's first comment to this answer:

What about an approach like this:

public interface IAccessor<K,V> {
    V this[K key] { get; }
}

public interface IAttributeDictionary<K,V> : IAccessor<K,V>, IDictionary<K,V> {
    // This interface just composes the other two.
}

public class Test<K,V> : IAttributeDictionary<K,V> {
    // This will implement the indexer for both IAccessor and IDictionary.
    // But when the object is accessed as an IAccessor the setter is not available.
    public V this[K key] {
        get { Console.WriteLine("getter"); return default(V); }
        set { Console.WriteLine("setter"); }
    }

    // ...the rest of IDictionary goes here...
}

class Program {
    static void Main (string[] args) {
        // Note that test can be accessed as any of the appropriate types,
        // and the same getter is called.
        Test<string,int> test = new Test<string, int>();
        int a = test["a"];
        int b = ((IDictionary<string, int>)test)["b"];
        int c = ((IAccessor<string, int>)test)["c"];
    }
}

EDIT 2.0: After all the discussion in the comments below, I think I might finally understand the issue, so...

I would argue that IAccessor really shouldn't use an indexer, since (in my opinion) the behavior you want out of it is quite unusual and unexpected. I would instead have GetValueForKey and ChangeValueForKey on IAccessor that can provide the behavior you want, and implement the indexer from IDictionary in the concrete implementation class. If that's unacceptable for any reason, I'd suggest then using explicit interface implementation to implement IAccessor and its indexer in the implementation class -- in both cases, I don't think the new declaration in IAttributeDictionary is necessary.