3
\$\begingroup\$

I was challenged to implement a mechanmism to support indexed properties in a class structure.

I wanted to avoid the need to implement an abstract property indexer class every time you want to implement a new T just because the backing store has changed. Semantically, if C# did provide indexed properties, the get/set logic would be on the class declaring the indexed property (as there would be no type).

So I implemented the following logic:

public interface IIndexedProperty<TKey, TValue>
{
    TValue this[TKey index] { get; set; }
}
public class IndexedProperty<TKey, TValue> : IIndexedProperty<TKey, TValue>
{
    private Func<TKey, TValue> Getter { get; set; }
    private Action<TKey, TValue> Setter { get; set; }
    public IndexedProperty(Func<TKey, TValue> getter, Action<TKey, TValue> setter)
    {
        this.Getter = getter;
        this.Setter = setter;
    }
    public TValue this[TKey index]
    {
        get
        {
            return Getter(index);
        }
        set
        {
            Setter(index, value);
        }
    }
}

An example implementation:

public class MyClass
{
    public IIndexedProperty<string, string> Values { get; private set; }
    private Dictionary<string, string> ValueDictionary { get; set; }
    public MyClass()
    {
        Values = new IndexedProperty<string, string>(index => ValueDictionary[index], (index, value) => ValueDictionary[index] = value);
        ValueDictionary = new Dictionary<string, string>();
    }
}

What he doesn't like about this is twofold:

  1. It isn't reusable (you have to implement the logic in your defining class each time you use it)
  2. It holds references to two delegates.

Can you see ways in which this can be improved?

\$\endgroup\$
2
\$\begingroup\$

This implementation allows for reusability of the indexer backing logic, as shown on MyClass two having two indexers using the same backing implementation. This pattern is much more reusable.

public abstract class PropertyIndexer<TKey, TValue>
{
    public abstract TValue this[TKey index] { get; set; }
}
public class MyClass
{
    private class StringDictionaryPropertyIndexer : PropertyIndexer<string, string>
    {
        private Dictionary<string, string> BackingCollection { get; set; }
        public StringDictionaryPropertyIndexer(Dictionary<string, string> backing)
        {
            this.BackingCollection = backing;
        }
        public override string this[string index]
        {
            get
            {
                string value;
                if (BackingCollection.TryGetValue(index, out value))
                    return value;
                return null;
            }
            set
            {
                BackingCollection[index] = value;
            }
        }
    }
    private Dictionary<string, string> data { get; set; }
    public PropertyIndexer<string, string> Data { get; private set; }
    private Dictionary<string, string> alternateData { get; set; }
    public PropertyIndexer<string, string> AlternateData { get; private set; }
    public MyClass()
    {
        data = new Dictionary<string, string>();
        Data = new StringDictionaryPropertyIndexer(data);
        alternateData = new Dictionary<string, string>();
        AlternateData = new StringDictionaryPropertyIndexer(alternateData);
    }
}
\$\endgroup\$
  • \$\begingroup\$ Could you explain the why of your changes a little better? \$\endgroup\$ – Hosch250 Feb 28 '15 at 21:02
  • \$\begingroup\$ You could implement my IIndexedProperty<TKey, TValue> on StringDictionaryPropertyIndexer and get the same semantics. \$\endgroup\$ – JasonLind Feb 28 '15 at 21:02
1
\$\begingroup\$

An old question, but I've been doing some work on this recently. Your question gave me a couple of ideas, so here are some in return.

Referencing Concerns

The issue of delegate references turns out to be a potential cause of memory leaks. This is true no matter what form of delegates you use, and doubly true of lambda expressions which close over anything you use, not just the class instance reference that delegates to member functions contain.

The first part of the solution to the referencing dilemma is to encapsulate your target object in a WeakReference and use either static methods or carefully crafted lambda expressions for the get/set delegates:

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

public class Indexer<TInstance, TKey, TValue> : IIndexer<TKey, TValue>
    where TInstance : class
{
    private readonly WeakReference _instance;
    private readonly Func<TInstance, TKey, TValue> _getter;
    private readonly Action<TInstance, TKey, TValue> _setter;

    public Indexer
        (
            TInstance instance, 
            Func<TInstance, TKey, TValue> getter, 
            Action<TInstance, TKey, TValue> setter
        )
    {
        _instance = new WeakReference(instance);
        _getter = getter;
        _setter = setter;
    }

    public TValue this[TKey key] 
    {
        get 
        {
            TInstance inst = _instance.Target as TInstance;
            if (inst != null)
                return _getter(inst, key);
            return default(TValue);
        }
        set 
        {
            TInstance inst = _instance.Target as TInstance;
            if (inst != null)
                _setter(inst, key, value); 
        }
    }
}

(Excuse the formatting, trying to avoid running wide.)

The WeakReference lets you hold a reference to an object without preventing it from being garbage collected. When you want to use the object (in the get or set) the IsAlive property will let you know if the target has been disposed of yet, but there is a potential race condition that could cause the object to be collected between checking and getting the value of the Target property of the reference. The code above bypasses that and works correctly when the object has been collected.

Indexer Code Reuse

The reuse issue is reasonably easy to manage with extension methods. Here are some example extensions for Dictionary:

public static class Extensions
{
    public static IIndexer<TKey, TValue> ToIndexer<TKey, TValue>
        (
            this Dictionary<TKey, TValue> target
        )
    {
        return new Indexer<Dictionary<TKey, TValue>, TKey, TValue>
            (
                target, 
                (instance, key) => 
                { 
                    TValue res = default(TValue);
                    instance.TryGetValue(key, out res);
                    return res;
                },
                (instance, key, value) => instance[key] = value
            );
    }

    public static IIndexer<TKey, TValue> ToIndexer<TKey, TValue>
        (
            this Dictionary<TKey, TValue> target, 
            Func<Dictionary<TKey, TValue>, TKey, TValue> getter,
            Action<Dictionary<TKey, TValue>, TKey, TValue> setter
        )
    {
        return new Indexer<Dictionary<TKey, TValue>, TKey, TValue>(target, getter, setter);
    }
}

The first uses a standard index on the dictionary instance, the second allows for custom implementations of the get and set methods.

Testing...

Here's a simple test case:

void Main()
{
    // the target of our indexer
    var dict = new Dictionary<string, object>();
    // create an indexer from the dictionary
    var vars = dict.ToIndexer();

    // assign and retrieve a value      
    vars["test"] = 123;
    Console.WriteLine(vars["test"]);

    // drop the dictionary reference and confirm it hasn't been garbage collected
    dict = null;
    Console.WriteLine(vars["test"]);

    // force garbage collection and confirm that the indexer doesn't keep it around
    GC.Collect();
    Console.WriteLine(vars["test"]);
}

Using extension methods greatly simplifies the construction of your indexers, and the IIndexer<TKey, TValue> is a much better signature than Indexer<Dictionary<TKey, TValue>, TKey, TValue> in my opinion. Certainly shorter to type.

References Revisited

Unfortunately this doesn't entirely solve the issue of lambda or delegate reference capture in user-provided get/set methods, which may be an intractable problem. The following will capture a reference in its lambdas:

public class Test
{
    private Dictionary<string, object> _vars = new Dictionary<string, object>();
    public IIndexer<string, object> Vars { get; private set; }

    private object get_Var(Dictionary<string, object> instance, string key)
    {
        object res = null;
        instance.TryGetValue(key, out res);
        return res;
    }
    private void set_Var(Dictionary<string, object> instance, string key, object value)
    {
        instance[key] = value;
    }

    public Test()
    {
        Vars = _vars.ToIndexer(get_Var, set_Var);
    }
}

The issue here is that get_Var and set_Var are non-static methods and will capture a reference to (or close over?) the Test instance and keep it alive and well. If the methods were static there would be fewer references bound to them.

Dealing with Bad Delegates

Lambda expressions are a little less likely to suffer from the same effect, but only if they do not access any other member of your class. If they do then they will also form a closure over those objects and retain references that will keep the garbage collector at bay.

So far the only method I've found for dealing with this is to make the IIndexer implement IDisposable and clear the getter and setter delegates to tidy up any references.

Here's the final product:

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

public class Indexer<TInstance, TKey, TValue> : IIndexer<TKey, TValue>
    where TInstance : class
{
    private WeakReference _instance;
    private Func<TInstance, TKey, TValue> _getter;
    private Action<TInstance, TKey, TValue> _setter;

    public Indexer(TInstance instance, Func<TInstance, TKey, TValue> getter, Action<TInstance, TKey, TValue> setter)
    {
        _instance = new WeakReference(instance);
        _getter = getter;
        _setter = setter;
    }

    public void Dispose()
    {
        _instance = null;
        _getter = null;
        _setter = null;
    }

    public TValue this[TKey key] 
    {
        get 
        {
            TInstance inst = _instance.Target as TInstance;
            if (inst != null)
                return _getter(inst, key);
            return default(TValue);
        }
        set 
        {
            TInstance inst = _instance.Target as TInstance;
            if (inst != null)
                _setter(inst, key, value); 
        }
    }
}

Needless to say you'll need to actually dispose of the indexers.

\$\endgroup\$

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.