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:
- It isn't reusable (you have to implement the logic in your defining class each time you use it)
- It holds references to two delegates.
Can you see ways in which this can be improved?
2 Answers 2
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);
}
}
-
\$\begingroup\$ Could you explain the why of your changes a little better? \$\endgroup\$user34073– user340732015年02月28日 21:02:07 +00:00Commented Feb 28, 2015 at 21:02
-
\$\begingroup\$ You could implement my IIndexedProperty<TKey, TValue> on StringDictionaryPropertyIndexer and get the same semantics. \$\endgroup\$JasonLind– JasonLind2015年02月28日 21:02:53 +00:00Commented Feb 28, 2015 at 21:02
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.