patterncsharpMinor
Proper use of a generic value type wrapper class
Viewed 0 times
genericvaluetypewrapperproperuseclass
Problem
I have a wrapper class:
The listener interface:
It's used in a program that alters save files of a game. An object is constructed using the save file data and any value types encountered are wrapped in the Value class. Object is then managed by the program and the save file is rewritten from the objects data upon saving.
The listener is here since altering some data in the save file influences other, so this gives me a way to manage that on a low level (instead of making TextBoxes notify each other).
When I save the file, I actually clone the object, process the clone for saving and save the clone. That's why the class is
My question is whether what I have done here is a well written code or should it be substituted with something else.
I also copied some of the # of references that VS displays just to demonstrate how heavily this class is used.
[Serializable]
// (465 references)
public class Value
{
private T value;
[NonSerialized]
private List> listeners = new List>();
// (232 references)
public Value(T value)
{
this.value = value;
}
// (274 references)
public T Get()
{
return value;
}
// (18 references)
public void Set(T value)
{
if (!this.value.Equals(value))
{
this.value = value;
foreach (IValueListener listener in listeners)
{
listener.ValueUpdated(this);
}
}
}
public void AddListener(IValueListener listener)
{
listeners.Add(listener);
}
public void RemoveListener(IValueListener listener)
{
listeners.Remove(listener);
}
}The listener interface:
public interface IValueListener
{
void ValueUpdated(Value source);
}It's used in a program that alters save files of a game. An object is constructed using the save file data and any value types encountered are wrapped in the Value class. Object is then managed by the program and the save file is rewritten from the objects data upon saving.
The listener is here since altering some data in the save file influences other, so this gives me a way to manage that on a low level (instead of making TextBoxes notify each other).
When I save the file, I actually clone the object, process the clone for saving and save the clone. That's why the class is
Serializable. I don't care about listeners when saving so the list of them isn't.My question is whether what I have done here is a well written code or should it be substituted with something else.
I also copied some of the # of references that VS displays just to demonstrate how heavily this class is used.
Solution
It is unclear how to use this class if I want to subscribe to two different objects that implement, say,
Also I think you should have an actual property instead of
Value. There is no way to tell which of the two values has changed. This looks like a pretty major design flaw, unless I am missing something. So you should probably either use regular events (so I can supply different handlers for different values) or provide "source" argument. Personally, I think first option is better. You should also consider using proper event aggregator instead of storing the list of listeners in values themselves.Also I think you should have an actual property instead of
Get() and Set() methods. And you can make listeners field read-only.Context
StackExchange Code Review Q#141475, answer score: 2
Revisions (0)
No revisions yet.