I have used a similiar approach at a time I needed it and in my opinion it is a valid way. That beeing said, let us take a look at the way you have implemented this.
public void SetAttr<T>(string name, T val) { var t = typeof(T); if (attributes.ContainsKey(t)) ((Dictionary<string, T>)attributes[t])[name] = val; else attributes[t] = new Dictionary<string, T>() { { name, val } }; }
The first improvement you should make is to avoid the call to the ContainsKey()
method. Performance wise it is better to use TryGetValue()
over ContainsKey()
.
See also: what-is-more-efficient-dictionary-trygetvalue-or-containskeyitem
The next I would always do is use brecaes {}
for single commend if..else..else if
statements to avoid stupid bugs.
Implementing the above would then lead to
public void SetAttr<T>(string name, T val)
{
var t = typeof(T);
object obj;
if (attributes.TryGetValue(t, out obj))
{
Dictionary<string, T> dictionary = obj as Dictionary<string, T>;
dictionary[name] = val;
}
else
{
attributes[t] = new Dictionary<string, T>() { { name, val } };
}
}
which makes it more clear what the object
in the attributes
dictionary is.
What should happen if you try to retrieve a value form the container which isn't present ? How about adding something like a TryGetValue<T>()
method to the container class.
Something like so
public bool TryGetAtrribute<T>(string name, out T val)
{
object obj;
if (attributes.TryGetValue(typeof(T), out obj))
{
Dictionary<string, T> dictionary = obj as Dictionary<string, T>;
if (dictionary != null)
{
val = dictionary[name];
return true;
}
}
val = default(T);
return false;
}
In this way it is more obvious for a different developer that the GetAttr<T>()
method can throw an exception.
Talking about exception, I would consider to throw an AttributeNotFoundException
for the case that the desired value isn't in the container.
I have used a similiar approach at a time I needed it and in my opinion it is a valid way. That beeing said, let us take a look at the way you have implemented this.
public void SetAttr<T>(string name, T val) { var t = typeof(T); if (attributes.ContainsKey(t)) ((Dictionary<string, T>)attributes[t])[name] = val; else attributes[t] = new Dictionary<string, T>() { { name, val } }; }
The first improvement you should make is to avoid the call to the ContainsKey()
method. Performance wise it is better to use TryGetValue()
over ContainsKey()
.
See also: what-is-more-efficient-dictionary-trygetvalue-or-containskeyitem
The next I would always do is use brecaes {}
for single commend if..else..else if
statements to avoid stupid bugs.
Implementing the above would then lead to
public void SetAttr<T>(string name, T val)
{
var t = typeof(T);
object obj;
if (attributes.TryGetValue(t, out obj))
{
Dictionary<string, T> dictionary = obj as Dictionary<string, T>;
dictionary[name] = val;
}
else
{
attributes[t] = new Dictionary<string, T>() { { name, val } };
}
}
which makes it more clear what the object
in the attributes
dictionary is.
What should happen if you try to retrieve a value form the container which isn't present ? How about adding something like a TryGetValue<T>()
method to the container class.
Something like so
public bool TryGetAtrribute<T>(string name, out T val)
{
object obj;
if (attributes.TryGetValue(typeof(T), out obj))
{
Dictionary<string, T> dictionary = obj as Dictionary<string, T>;
val = dictionary[name];
return true;
}
val = default(T);
return false;
}
In this way it is more obvious for a different developer that the GetAttr<T>()
method can throw an exception.
Talking about exception, I would consider to throw an AttributeNotFoundException
for the case that the desired value isn't in the container.
I have used a similiar approach at a time I needed it and in my opinion it is a valid way. That beeing said, let us take a look at the way you have implemented this.
public void SetAttr<T>(string name, T val) { var t = typeof(T); if (attributes.ContainsKey(t)) ((Dictionary<string, T>)attributes[t])[name] = val; else attributes[t] = new Dictionary<string, T>() { { name, val } }; }
The first improvement you should make is to avoid the call to the ContainsKey()
method. Performance wise it is better to use TryGetValue()
over ContainsKey()
.
See also: what-is-more-efficient-dictionary-trygetvalue-or-containskeyitem
The next I would always do is use brecaes {}
for single commend if..else..else if
statements to avoid stupid bugs.
Implementing the above would then lead to
public void SetAttr<T>(string name, T val)
{
var t = typeof(T);
object obj;
if (attributes.TryGetValue(t, out obj))
{
Dictionary<string, T> dictionary = obj as Dictionary<string, T>;
dictionary[name] = val;
}
else
{
attributes[t] = new Dictionary<string, T>() { { name, val } };
}
}
which makes it more clear what the object
in the attributes
dictionary is.
What should happen if you try to retrieve a value form the container which isn't present ? How about adding something like a TryGetValue<T>()
method to the container class.
Something like so
public bool TryGetAtrribute<T>(string name, out T val)
{
object obj;
if (attributes.TryGetValue(typeof(T), out obj))
{
Dictionary<string, T> dictionary = obj as Dictionary<string, T>;
if (dictionary != null)
{
val = dictionary[name];
return true;
}
}
val = default(T);
return false;
}
In this way it is more obvious for a different developer that the GetAttr<T>()
method can throw an exception.
Talking about exception, I would consider to throw an AttributeNotFoundException
for the case that the desired value isn't in the container.
I have used a similiar approach at a time I needed it and in my opinion it is a valid way. That beeing said, let us take a look at the way you have implemented this.
public void SetAttr<T>(string name, T val) { var t = typeof(T); if (attributes.ContainsKey(t)) ((Dictionary<string, T>)attributes[t])[name] = val; else attributes[t] = new Dictionary<string, T>() { { name, val } }; }
The first improvement you should make is to avoid the call to the ContainsKey()
method. Performance wise it is better to use TryGetValue()
over ContainsKey()
.
See also: what-is-more-efficient-dictionary-trygetvalue-or-containskeyitem
The next I would always do is use brecaes {}
for single commend if..else..else if
statements to avoid stupid bugs.
Implementing the above would then lead to
public void SetAttr<T>(string name, T val)
{
var t = typeof(T);
object obj;
if (attributes.TryGetValue(t, out obj))
{
Dictionary<string, T> dictionary = obj as Dictionary<string, T>;
dictionary[name] = val;
}
else
{
attributes[t] = new Dictionary<string, T>() { { name, val } };
}
}
which makes it more clear what the object
in the attributes
dictionary is.
What should happen if you try to retrieve a value form the container which isn't present ? How about adding something like a TryGetValue<T>()
method to the container class.
Something like so
public bool TryGetAtrribute<T>(string name, out T val)
{
object obj;
if (attributes.TryGetValue(typeof(T), out obj))
{
Dictionary<string, T> dictionary = obj as Dictionary<string, T>;
val = dictionary[name];
return true;
}
val = default(T);
return false;
}
In this way it is more obvious for a different developer that the GetAttr<T>()
method can throw an exception.
Talking about exception, I would consider to throw an AttributeNotFoundException
for the case that the desired value isn't in the container.