Code Review Asked on December 12, 2021
Assume the following situation: you have an object that can store any object based on a key (basically, IDictionary<string, object>
). You want to store objects of various types into it that are not directly related. (For example, the dictionary can be an ASP.NET Session
, or it can represent a dictionary that will be serialized to disk for persistent storage.)
I don’t want to create a single class that will contain all those objects, because they are not directly related, they come from different places. But if you store each object separately, it means you have to use casts when getting some value and there is no type-check when you’re setting it.
To solve this, I created a generic type that encapsulates the key along with the associated type and a couple of extension methods that use it:
class TypedKey<T>
{
public string Name { get; private set; }
public TypedKey(string name)
{
Name = name;
}
}
static class DictionaryExtensions
{
public static T Get<T>(this IDictionary<string, object> dictionary, TypedKey<T> key)
{
return (T)dictionary[key.Name];
}
public static void Set<T>(this IDictionary<string, object> dictionary, TypedKey<T> key, T value)
{
dictionary[key.Name] = value;
}
}
Usage:
private static readonly TypedKey<int> AgeKey = new TypedKey<int>("age");
…
dictionary.Get(AgeKey) > 18
dictionary.Set(AgeKey, age)
This has the type-safety (both on get and set) of using a property, while being backed by a dictionary that can store anything.
What do you think about this pattern?
I have spent a long time grappling with this problem. I have finally come to the conclusion that it actually is a non-problem!
The thing is that once you have pushed the object into a collection you have lost the compile time binding. Type safety is really a compile time issue rather than a runtime issue.
My understanding is that when you create a generic the compiler creates new methods to call depending on the generic type passed. So if you have a method
public void Foo<T>(){}
and call it using
Foo<int>();
then
Foo<string>();
the compiler creates 2 methods (I have used pseudo code signatures for the sake of accessibility.)
public Foo<int>(){}
public Foo<string>(){}
If you assign methods to aggregate these to a collection for retrieval you the compiler cannot know what the type of the object that is retrieved from the collection is. So you can no longer have type safety.
The upshot of this is no matter how many hoops you try to jump through to make the generic store undefined types, it cannot be done. I have come to the conclusion it is better to store the type that was saved explicitly, rather than creating an overload that is actually being used as a type member and explicitly store the types separately in a collection for validation at runtime. The resulting code is leaner and can be understood without knowledge of any 'patterns'.
I do not think it is impossible to do, but it would require a significant language change to c# and one or more new object types to facilitate it. I am more than open to correction on any of this.
Preferable to use Dictionary<T,object> then cast the returned object. so you would have
example 1
var dic = new Dictionary <string, object>();
string item1="item1";
int item2=2;
dic.Add("item1",item1);
dic.Add("item2",item2);
var retrievedItem1=(string)dic["item1"];
var retrievedItem2=(int)dic["item2"];
compared to example 2
var dic = new DictionaryWithLotsOfExtraFunkyCode <string>();
string item1="item1";
int item2=2;
dic.Add("item1",item1);
dic.Add("item2",item2);
var retrievedItem1=dic.Get<string>("item1");
var retrievedItem2=dic.Get<int>("item2");
There is no extra type safety (both will compile and both will throw runtime errors if attempting to access the wrong type). In example 2 there is just extra code to maintain.
Answered by symaps on December 12, 2021
You could allow for some more flexibility in type conversions by using type converters.
public static T Get<T>(this IDictionary<string, object> dictionary, TypedKey<T> key)
{
return (T)Convert.ChangeType(dictionary[key.Name], typeof(T));
}
static class DictionaryExtensions { public static T Get<T>(this IDictionary<string, object> dictionary, TypedKey<T> key) { return (T)dictionary[key.Name]; } public static void Set<T>(this IDictionary<string, object> dictionary, TypedKey<T> key, T value) { dictionary[key.Name] = value; } }
Edit (after remark OP)
The following operations make more sense. They can be overloads next to the existing original Get
.
public static V Get<T, V>(this IDictionary<string, object> dictionary, TypedKey<T> key)
{
return (V)Convert.ChangeType(dictionary[key.Name], typeof(T));
}
public static T Get<T>(this IDictionary<string, object> dictionary, string key)
{
return (T)Convert.ChangeType(dictionary[key], typeof(T));
}
Answered by dfhwze on December 12, 2021
I know that this solution has some really bumpy corners and using it at scale would have some really interesting implications, but please consider it as a thought experiment.
In all of the previously posted examples you still have the potential for a mistake - i.e., someone might intentionally or unintentionally do something along the lines of this:
private static readonly TypedKey<int> AgeKey = new TypedKey<int>("age");
private static readonly TypedKey<string> BadAgeKey = new TypedKey<string>("age");
dictionary.Set(BadAgeKey, “foo”);
...
// this would throw
dictionary.Get(AgeKey);
And there is really nothing that you can do at compile time to validate that. You could implement some kind of code analyzer to look for examples of AgeKey
and BadAgeKey
. Normally this is addressed by name spacing keys so that they don’t overlap.
In this solution, I attempted to solve that problem… To do that I dropped the string key, and instead indexed the dictionary by Type, especially the Type of what the consumer is using as a key, and not the Type of data being stored. Doing that means that each key has to be a predefined Type. It does give you options for accessing the data.
You could select which key to read/write at compile time using a Generic Type parameter or you could defer that to run time by passing the key as a normal parameter.
And I came up with this:
public class TypeSafeKey<T> { }
public class TypeSafeKeyValuePairBag
{
public T GetItemOrDefault<TKey, T>(T defaultValue = default(T)) where TKey : TypeSafeKey<T>
=> TryGet(typeof(TKey), out T result) ? result : defaultValue;
public T GetItemOrDefault<T>(TypeSafeKey<T> key, T defaultValue = default(T))
=> TryGet(key?.GetType() ?? throw new ArgumentNullException(nameof(key)), out T result) ? result : defaultValue;
public void SetItem<TKey, T>(T value) where TKey : TypeSafeKey<T>
=> m_values[typeof(TKey)] = value;
public void SetItem<T>(TypeSafeKey<T> key, T value)
=> m_values[key?.GetType() ?? throw new ArgumentNullException(nameof(key))] = value;
public T GetItem<TKey, T>() where TKey : TypeSafeKey<T>
=> Get<T>(typeof(TKey));
public T GetItem<T>(TypeSafeKey<T> key)
=> Get<T>(key?.GetType() ?? throw new ArgumentNullException(nameof(key)));
private bool TryGet<T>(Type type, out T value)
{
if (m_values.TryGetValue(type, out object obj))
{
value = (T)obj;
return true;
}
value = default(T);
return false;
}
private T Get<T>(Type type)
=> TryGet(type, out T result) ? result : throw new KeyNotFoundException($"Key {type.FullName} not found");
private Dictionary<Type, object> m_values = new Dictionary<Type, object>();
}
Then using it would look something like this:
// You need to declare a Type for each key that you want to use, all though the types can defined anywhere
// They don't need to be known to the assembly where TypeSafeKeyValuePairBag is defined, but they do need to
// be known to the any code that is setting or getting any given key. So even though the declaration of
// these class could be spread throughout the source tree, since each is a type they are forced to be unique
public class KeyHight : TypeSafeKey<int> { }
public class KeyWidth : TypeSafeKey<int> { }
public class KeyName : TypeSafeKey<string> { }
// A static class, with static public members would reduce the number of instances of objects that needed to be created for repeated reads/writes.
// You would need to create these in a lazy fashion if you had many of them. And since only their type matters, you don’t need to worry about locking, since two different instances of the same Type would function as the same key.
public static class Keys
{
public static KeyHight KeyHight { get; } = new KeyHight();
public static KeyWidth KeyWidth { get; } = new KeyWidth();
public static KeyName KeyName { get; } = new KeyName();
}
...
TypeSafeKeyValuePairBag bag = new TypeSafeKeyValuePairBag();
// Accessing hard coded keys
//Using Generic Type Parameters: The compiler can't infer the value Type from the Key Type, which means listing them both
bag.SetItem<KeyHight, int>(5);
//Passing the key as a parameter
bag.SetItem(Keys.KeyWidth, 10);
bag.SetItem(Keys.KeyName, "foo");
// Selecting which keys to access at run time
int value = 1;
foreach(var key in new TypeSafeKey<int>[] { Keys.KeyHight, Keys.KeyWidth })
{
value *= bag.GetItem(key);
}
Console.WriteLine($"{bag.GetItem<KeyName, string>()}'s area is {value}");
This does have some large drawbacks. Specifically you need to create a lot of types which will add bloat. And serializing it would be rather verbose. It would also still be easy to use the wrong key, and before where that may have resulted in a runtime error, the wrong key could easily lead to corruption.
Using Types has another really large complication if you are using versioned software. Many of the previous examples could easily be shared in a process that has a mixture of different assembly versions loaded. This is definitely true if all the items in the property bag were primitive types. The string constant “foo” defined in version 1.0 is going to the same in version 2.0 (assuming no one change the value). When you start using types as keys you will get unexpected results if someone sets a value with one version of the key, but attempts to read it with another, specifically KeyHight defined in version 1.0 is not the same type as KeyHeight defined in version 2.0
The multiple ways of getting to the setters and getters could also make it difficult to locate all the uses of a specific key, but since they are bound to type, most IDEs can easily get you a comprehensive list of accesses.
I spent some time exploring what else you could do if you had a structure like this. You could utilize the inheritance to build a hierarchy. There are likely simpler and more efficient way to do this, but consider if we change the TryGet method like this:
private bool TryGet<T>(Type type, out T value)
{
if (m_values.TryGetValue(type, out object obj))
{
value = (T)obj;
return true;
}
Type baseType = type.BaseType;
if (baseType != typeof(TypeSafeKey<T>))
{
return TryGet(baseType, out value);
}
value = default(T);
return false;
}
And you consumed it like this:
public class KeyLevel0Value : TypeSafeKey<int> { }
public class KeyLevelA1Value : KeyLevel0Value { }
public class KeyLevelA2Value : KeyLevelA1Value { }
public class KeyLevelB1Value : KeyLevel0Value { }
public class KeyLevelB2Value : KeyLevelB1Value { }
...
bag.SetItem<KeyLevelA1Value, int>(5);
// This will first check the value for LevelA2. After not finding it, it will check LevelA2, and that value will be returned
Console.WriteLine(bag.GetItem<KeyLevelA2Value, int>());
// This will first check the value for LevelB2, LevelB1, and finally Level0, and since none are set it will return default
Console.WriteLine(bag.GetItemOrDefault<KeyLevelB2Value, int>());
So with all that being said, this does create a Property bag that can hold arbitrary types, and access to the data is done in a type safe way.
Answered by miholmes on December 12, 2021
I know you don't want to create a single class, but this seems exactly what is needed. I would create a new class and favor composition. Call the whole ball of wax a PropertyBag
since that declares its intent a bit clearer. I also am a fan of interfaced-based development, so I extracted a couple of them. Note one constructor overload takes a non-generic IDictionary
so you can create one of these from any existing dictionary (generic or not). Commentary welcome.
public interface ITypedKey<T>
{
string Name { get; }
}
public class TypedKey<T> : ITypedKey<T>
{
public TypedKey(string name) => this.Name = name ?? throw new ArgumentNullException(nameof(name));
public string Name { get; }
}
public interface IPropertyBag
{
T Get<T>(ITypedKey<T> key);
bool TryGet<T>(ITypedKey<T> key, out T value);
void Set<T>(ITypedKey<T> key, T value);
void Remove<T>(ITypedKey<T> key);
}
public class PropertyBag : IPropertyBag
{
private readonly IDictionary<string, object> _bag;
public PropertyBag() => this._bag = new Dictionary<string, object>();
public PropertyBag(IDictionary dict)
{
if (dict == null)
{
throw new ArgumentNullException(nameof(dict));
}
this._bag = new Dictionary<string, object>(dict.Count);
foreach (DictionaryEntry kvp in dict)
{
this._bag.Add(new KeyValuePair<string, object>(kvp.Key.ToString(), kvp.Value));
}
}
public T Get<T>(ITypedKey<T> key)
{
if (key == null)
{
throw new ArgumentNullException(nameof(key));
}
return (T)this._bag[key.Name];
}
public bool TryGet<T>(ITypedKey<T> key, out T value)
{
if (this._bag.TryGetValue(key.Name, out object result) && result is T typedValue)
{
value = typedValue;
return true;
}
value = default(T);
return false;
}
public void Set<T>(ITypedKey<T> key, T value)
{
if (key == null)
{
throw new ArgumentNullException(nameof(key));
}
this._bag[key.Name] = value;
}
public void Remove<T>(ITypedKey<T> key)
{
if (key == null)
{
throw new ArgumentNullException(nameof(key));
}
this._bag.Remove(key.Name);
}
}
Answered by Jesse C. Slicer on December 12, 2021
Your suggestion is not really type-safe as you can still pass a key of the wrong type. Therefore I would just use a normal (string) key. But I would add a generic TryGet
method which takes account of the type. The setter needs not to be generic.
static class DictionaryExtensions
{
public static T Get<T>(this IDictionary<string, object> dictionary, string key)
{
return (T)dictionary[key];
}
public static bool TryGet<T>(this IDictionary<string, object> dictionary,
string key, out T value)
{
object result;
if (dictionary.TryGetValue(key, out result) && result is T) {
value = (T)result;
return true;
}
value = default(T);
return false;
}
public static void Set(this IDictionary<string, object> dictionary,
string key, object value)
{
dictionary[key] = value;
}
}
You can then use the dictionary like this.
int age = 20;
dictionary.Set("age", age);
// ...
age = dictionary.Get<int>("age");
// or the safe way
if (dictionary.TryGet("age", out age)) {
Console.WriteLine("The age is {0}", age);
} else {
Console.WriteLine("Age not found or of wrong type");
}
Note that the compiler can infer the generic type when using TryGet
.
UPDATE
In despite of my suggestion above, I must agree that your solution is elegant. Here is another suggestion which is based on your solution but which encapsulates the dictionary instead of providing a key. Well, it acts as wrapper and as key at the same time
public class Property<T>
{
Dictionary<object, object> _dict;
public Property (Dictionary<object, object> dict)
{
_dict = dict;
}
public T Value {
get { return (T)_dict[this]; }
set { _dict[this] = value; }
}
}
Alternatively, a string key could be provided in the Property's constructor.
You can use it like this
private static readonly Dictionary<object, object> _properties =
new Dictionary<object, object>();
private static readonly Property<int> _age = new Property<int>(_properties);
...
_age.Value > 18
_age.Value = age
Answered by Olivier Jacot-Descombes on December 12, 2021
Get help from others!
Recent Questions
Recent Answers
© 2024 TransWikia.com. All rights reserved. Sites we Love: PCI Database, UKBizDB, Menu Kuliner, Sharing RPP