patterncsharpMinor
Listening for values changed by the memory
Viewed 0 times
theforlisteningmemoryvalueschanged
Problem
I'm fetching some values from the memory. I would like to create some custom events that I can listen to for each value. Since there are no existing events regarding memory changes, I've decided to go with a polling solution. I'm restarting the timer below manually just to make certain the work is finished.
Would this be an okay solution (considering the circumstances) to my problem? Do you see any issues with creating multiple timers? As far as I know, each instance will run on its own thread.
This was the best I could come up with right off the bat. I would like to hear your input regarding possible improvements:
Would this be an okay solution (considering the circumstances) to my problem? Do you see any issues with creating multiple timers? As far as I know, each instance will run on its own thread.
This was the best I could come up with right off the bat. I would like to hear your input regarding possible improvements:
var vc = new ValueChange(1000, Blah.GetMemoryValue());
vc.PropertyChanged += OnPropertyChanged;
void OnPropertyChanged(object sender, PropertyChangedEventArgs e)
{
Console.WriteLine("prop change");
}
public class ValueChange : INotifyPropertyChanged
{
private int _val;
private readonly System.Timers.Timer _timer;
private int Value
{
set
{
_val = value;
RaisePropertyChanged("Val");
}
get
{
return _val;
}
}
public ValueChange(double polingInterval, int val)
{
_val = val;
_timer = new System.Timers.Timer { AutoReset = false, Interval = polingInterval };
_timer.Elapsed += TimerElapsed;
_timer.Start();
}
private void TimerElapsed(object sender, ElapsedEventArgs e)
{
var newValue = Blah.GetMemoryValue();
if (Value != newValue)
{
Value = newValue;
}
_timer.Start();
}
public event PropertyChangedEventHandler PropertyChanged;
private void RaisePropertyChanged(string caller)
{
if (PropertyChanged != null)
{
PropertyChanged(this, new PropertyChangedEventArgs(caller));
}
}
}Solution
-
Having a private property is quite unusual. It's especially weird since you only use it once in
-
Why is the initial value passed in the constructor? I think the class should retrieve the initial value by itself.
-
-
Your code is a prime candidate for making it more generic: you could change
Also,
-
I like to avoid
But this is not really a common technique.
-
Creating multiple timers is okay, they execute on thread pool threads. This means that if several timers tick at the same time, they will be (most likely) executed concurrently by different threads.
-
In general, polling like this is not ideal. It would be better if you could get a notification whenever the value actually changes. But it seems like this is not possible in your case.
-
You shouldn't shorten the names of your variables. So,
Having a private property is quite unusual. It's especially weird since you only use it once in
TimerElapsed. I would probably just use the field directly and call RaisePropertyChanged() manually when necessary.-
Why is the initial value passed in the constructor? I think the class should retrieve the initial value by itself.
-
PropertyChanged should be raised for a property that's readable from the outside and the name in the event args should be the same as the name of that property.-
Your code is a prime candidate for making it more generic: you could change
ValueChange to ValueChange and int _val to T _val. (But if you do this, be careful with equality, !object.Equals(_val, newValue) is probably the simplest way to do it correctly.)Also,
Blah.GetMemoryValue should be configurable by the caller. You could achieve this by accepting a Func delegate in the constructor. Something like:private readonly Func _getValue;
public ValueChange(double polingInterval, Func getValue)
{
_getValue = getValue;
…
}
private void TimerElapsed(object sender, ElapsedEventArgs e)
{
var newValue = _getValue();
…
}-
I like to avoid
null checks on events by making sure there always is at least one subscriber:public event PropertyChangedEventHandler PropertyChanged = delegate { };But this is not really a common technique.
-
Creating multiple timers is okay, they execute on thread pool threads. This means that if several timers tick at the same time, they will be (most likely) executed concurrently by different threads.
-
In general, polling like this is not ideal. It would be better if you could get a notification whenever the value actually changes. But it seems like this is not possible in your case.
-
You shouldn't shorten the names of your variables. So,
_value is better than just _val.Code Snippets
private readonly Func<T> _getValue;
public ValueChange(double polingInterval, Func<T> getValue)
{
_getValue = getValue;
…
}
private void TimerElapsed(object sender, ElapsedEventArgs e)
{
var newValue = _getValue();
…
}public event PropertyChangedEventHandler PropertyChanged = delegate { };Context
StackExchange Code Review Q#35845, answer score: 3
Revisions (0)
No revisions yet.