Limit and merge saving operations
Clash Royale CLAN TAG#URR8PPP
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;
up vote
2
down vote
favorite
Use case:
I want to automatically save data in the storage when a property of an object that implements INotifyPropertyChanged changes. Therefore I subscribe to the PropertyChanged event and call a method that saves the object.
Problem:
It can be that a lot of properties change over a short period of time and that is a problem because the saving method is an I/O operation.
My solution:
Accumulate the save orders over a period of time in a singleton class and perform the saving operations for the last order.
Code:
internal sealed class AccumulatedSaver
private readonly Queue<Tuple<string, object>> _saveOrders = new Queue<Tuple<string, object>>();
private bool _busy = false;
internal static AccumulatedSaver Instance get; = new AccumulatedSaver();
internal async Task SaveToAppData<T>(T item, string fileName)
_saveOrders.Enqueue(new Tuple<string, object>(fileName,item));
if (_busy)
return;
_busy = true;
await Task.Run(async() => await Task.Delay(500));
while (_saveOrders.Count > 0)
Tuple<string, object> saveOrder = _saveOrders.Dequeue();
if (_saveOrders.All(o => o.Item1 != saveOrder.Item1))
await SaveHelper.TrySaveToAppData(saveOrder.Item2, saveOrder.Item1);
_busy = false;
Question:
Is that a good idea to do that like that?
What can be made better?
c# .net uwp
add a comment |Â
up vote
2
down vote
favorite
Use case:
I want to automatically save data in the storage when a property of an object that implements INotifyPropertyChanged changes. Therefore I subscribe to the PropertyChanged event and call a method that saves the object.
Problem:
It can be that a lot of properties change over a short period of time and that is a problem because the saving method is an I/O operation.
My solution:
Accumulate the save orders over a period of time in a singleton class and perform the saving operations for the last order.
Code:
internal sealed class AccumulatedSaver
private readonly Queue<Tuple<string, object>> _saveOrders = new Queue<Tuple<string, object>>();
private bool _busy = false;
internal static AccumulatedSaver Instance get; = new AccumulatedSaver();
internal async Task SaveToAppData<T>(T item, string fileName)
_saveOrders.Enqueue(new Tuple<string, object>(fileName,item));
if (_busy)
return;
_busy = true;
await Task.Run(async() => await Task.Delay(500));
while (_saveOrders.Count > 0)
Tuple<string, object> saveOrder = _saveOrders.Dequeue();
if (_saveOrders.All(o => o.Item1 != saveOrder.Item1))
await SaveHelper.TrySaveToAppData(saveOrder.Item2, saveOrder.Item1);
_busy = false;
Question:
Is that a good idea to do that like that?
What can be made better?
c# .net uwp
add a comment |Â
up vote
2
down vote
favorite
up vote
2
down vote
favorite
Use case:
I want to automatically save data in the storage when a property of an object that implements INotifyPropertyChanged changes. Therefore I subscribe to the PropertyChanged event and call a method that saves the object.
Problem:
It can be that a lot of properties change over a short period of time and that is a problem because the saving method is an I/O operation.
My solution:
Accumulate the save orders over a period of time in a singleton class and perform the saving operations for the last order.
Code:
internal sealed class AccumulatedSaver
private readonly Queue<Tuple<string, object>> _saveOrders = new Queue<Tuple<string, object>>();
private bool _busy = false;
internal static AccumulatedSaver Instance get; = new AccumulatedSaver();
internal async Task SaveToAppData<T>(T item, string fileName)
_saveOrders.Enqueue(new Tuple<string, object>(fileName,item));
if (_busy)
return;
_busy = true;
await Task.Run(async() => await Task.Delay(500));
while (_saveOrders.Count > 0)
Tuple<string, object> saveOrder = _saveOrders.Dequeue();
if (_saveOrders.All(o => o.Item1 != saveOrder.Item1))
await SaveHelper.TrySaveToAppData(saveOrder.Item2, saveOrder.Item1);
_busy = false;
Question:
Is that a good idea to do that like that?
What can be made better?
c# .net uwp
Use case:
I want to automatically save data in the storage when a property of an object that implements INotifyPropertyChanged changes. Therefore I subscribe to the PropertyChanged event and call a method that saves the object.
Problem:
It can be that a lot of properties change over a short period of time and that is a problem because the saving method is an I/O operation.
My solution:
Accumulate the save orders over a period of time in a singleton class and perform the saving operations for the last order.
Code:
internal sealed class AccumulatedSaver
private readonly Queue<Tuple<string, object>> _saveOrders = new Queue<Tuple<string, object>>();
private bool _busy = false;
internal static AccumulatedSaver Instance get; = new AccumulatedSaver();
internal async Task SaveToAppData<T>(T item, string fileName)
_saveOrders.Enqueue(new Tuple<string, object>(fileName,item));
if (_busy)
return;
_busy = true;
await Task.Run(async() => await Task.Delay(500));
while (_saveOrders.Count > 0)
Tuple<string, object> saveOrder = _saveOrders.Dequeue();
if (_saveOrders.All(o => o.Item1 != saveOrder.Item1))
await SaveHelper.TrySaveToAppData(saveOrder.Item2, saveOrder.Item1);
_busy = false;
Question:
Is that a good idea to do that like that?
What can be made better?
c# .net uwp
asked Feb 27 at 16:08
HIG Dev
132
132
add a comment |Â
add a comment |Â
2 Answers
2
active
oldest
votes
up vote
1
down vote
accepted
Alternative: Rx
Creating your own producer/consumer solution is not always easy and is one option but you could also use the tools that are already there. One such tool are the Reactive Extensions (NuGet-Package: System.Reactive
). It can be very helpful in your scenario.
I requires two parties
- an observable and
- an observer
Your not yet real observables are the POCO objects that implement the INotifyPropertyChanged
interface. You can turn them into observables with helper factories that can derive an IObservable
from the PropertyChanged
event pattern.
class Person : INotifyPropertyChanged, IObservable<EventPattern<PropertyChangedEventArgs>>
private readonly IObservable<EventPattern<PropertyChangedEventArgs>> _observable;
private string _name;
public Person()
_observable =
Observable
.FromEventPattern<PropertyChangedEventHandler, PropertyChangedEventArgs>(
handler => PropertyChanged += handler,
handler => PropertyChanged -= handler
);
public event PropertyChangedEventHandler PropertyChanged;
public string Name
get => _name;
set
_name = value;
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Name)));
public IDisposable Subscribe(IObserver<EventPattern<PropertyChangedEventArgs>> observer)
return _observable.Subscribe(observer);
Now you only need a task to process a collection of them. You can create it in many different ways so here is just one of them. I created an observable from an array of persons that I Merge
into a single observable that I want to watch. I'd like this to be processed either every ten items or every two seconds (there are also other options). Then each batch is processed with the ForEachAsync
loop.
var persons = new
new Person Name = "John",
new Person Name = "Tom"
;
var savePersonBatchTask =
persons
.Merge()
.Buffer(timeSpan: TimeSpan.FromSeconds(2), count: 10)
.Where(batch => buffer.Count > 0)
.ForEachAsync(batch =>
Console.WriteLine($"New batch arrived: batch.Count");
);
You can test this simulation by creating another loop that modifies the person's name 27 times.
for (int i = 0; i < 27; i++)
foreach (var person in persons)
person.Name = i.ToString();
await savePersonBatchTask;
The output will look like this
New batch arrived: 10
New batch arrived: 10
New batch arrived: 10
New batch arrived: 10
New batch arrived: 10
New batch arrived: 4
There is a lot more it can do and all that is explained very nicely on Introduction to Rx.
Review
And I also have a couple of comments about your code...
private readonly Queue<Tuple<string, object>> _saveOrders
You can simplify this and make it easier to use by using strong tuples with named properties. All you need is to throw away the Tuple
type and let the compiler derive the tuple for you.
private readonly Queue<(string FileName, object Item)> _saveOrders = ..;
later in code you also don't need it anymore
saveOrders.Enqueue((fileName,item));
Your while
loop will become simpler too because tuples can be deconstructed into variables:
while (_saveOrders.Count > 0)
var (fileName, item) = _saveOrders.Dequeue();
if (_saveOrders.All(o => o.FileName != fileName))
await SaveHelper.TrySaveToAppData(fileName, item);
add a comment |Â
up vote
0
down vote
Problem I see is no call to force the save. You could have that in another method but you should not be repeating the save code.
Seems to me a producer consumer like BackGroundWorker would be a better approach here.
add a comment |Â
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
1
down vote
accepted
Alternative: Rx
Creating your own producer/consumer solution is not always easy and is one option but you could also use the tools that are already there. One such tool are the Reactive Extensions (NuGet-Package: System.Reactive
). It can be very helpful in your scenario.
I requires two parties
- an observable and
- an observer
Your not yet real observables are the POCO objects that implement the INotifyPropertyChanged
interface. You can turn them into observables with helper factories that can derive an IObservable
from the PropertyChanged
event pattern.
class Person : INotifyPropertyChanged, IObservable<EventPattern<PropertyChangedEventArgs>>
private readonly IObservable<EventPattern<PropertyChangedEventArgs>> _observable;
private string _name;
public Person()
_observable =
Observable
.FromEventPattern<PropertyChangedEventHandler, PropertyChangedEventArgs>(
handler => PropertyChanged += handler,
handler => PropertyChanged -= handler
);
public event PropertyChangedEventHandler PropertyChanged;
public string Name
get => _name;
set
_name = value;
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Name)));
public IDisposable Subscribe(IObserver<EventPattern<PropertyChangedEventArgs>> observer)
return _observable.Subscribe(observer);
Now you only need a task to process a collection of them. You can create it in many different ways so here is just one of them. I created an observable from an array of persons that I Merge
into a single observable that I want to watch. I'd like this to be processed either every ten items or every two seconds (there are also other options). Then each batch is processed with the ForEachAsync
loop.
var persons = new
new Person Name = "John",
new Person Name = "Tom"
;
var savePersonBatchTask =
persons
.Merge()
.Buffer(timeSpan: TimeSpan.FromSeconds(2), count: 10)
.Where(batch => buffer.Count > 0)
.ForEachAsync(batch =>
Console.WriteLine($"New batch arrived: batch.Count");
);
You can test this simulation by creating another loop that modifies the person's name 27 times.
for (int i = 0; i < 27; i++)
foreach (var person in persons)
person.Name = i.ToString();
await savePersonBatchTask;
The output will look like this
New batch arrived: 10
New batch arrived: 10
New batch arrived: 10
New batch arrived: 10
New batch arrived: 10
New batch arrived: 4
There is a lot more it can do and all that is explained very nicely on Introduction to Rx.
Review
And I also have a couple of comments about your code...
private readonly Queue<Tuple<string, object>> _saveOrders
You can simplify this and make it easier to use by using strong tuples with named properties. All you need is to throw away the Tuple
type and let the compiler derive the tuple for you.
private readonly Queue<(string FileName, object Item)> _saveOrders = ..;
later in code you also don't need it anymore
saveOrders.Enqueue((fileName,item));
Your while
loop will become simpler too because tuples can be deconstructed into variables:
while (_saveOrders.Count > 0)
var (fileName, item) = _saveOrders.Dequeue();
if (_saveOrders.All(o => o.FileName != fileName))
await SaveHelper.TrySaveToAppData(fileName, item);
add a comment |Â
up vote
1
down vote
accepted
Alternative: Rx
Creating your own producer/consumer solution is not always easy and is one option but you could also use the tools that are already there. One such tool are the Reactive Extensions (NuGet-Package: System.Reactive
). It can be very helpful in your scenario.
I requires two parties
- an observable and
- an observer
Your not yet real observables are the POCO objects that implement the INotifyPropertyChanged
interface. You can turn them into observables with helper factories that can derive an IObservable
from the PropertyChanged
event pattern.
class Person : INotifyPropertyChanged, IObservable<EventPattern<PropertyChangedEventArgs>>
private readonly IObservable<EventPattern<PropertyChangedEventArgs>> _observable;
private string _name;
public Person()
_observable =
Observable
.FromEventPattern<PropertyChangedEventHandler, PropertyChangedEventArgs>(
handler => PropertyChanged += handler,
handler => PropertyChanged -= handler
);
public event PropertyChangedEventHandler PropertyChanged;
public string Name
get => _name;
set
_name = value;
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Name)));
public IDisposable Subscribe(IObserver<EventPattern<PropertyChangedEventArgs>> observer)
return _observable.Subscribe(observer);
Now you only need a task to process a collection of them. You can create it in many different ways so here is just one of them. I created an observable from an array of persons that I Merge
into a single observable that I want to watch. I'd like this to be processed either every ten items or every two seconds (there are also other options). Then each batch is processed with the ForEachAsync
loop.
var persons = new
new Person Name = "John",
new Person Name = "Tom"
;
var savePersonBatchTask =
persons
.Merge()
.Buffer(timeSpan: TimeSpan.FromSeconds(2), count: 10)
.Where(batch => buffer.Count > 0)
.ForEachAsync(batch =>
Console.WriteLine($"New batch arrived: batch.Count");
);
You can test this simulation by creating another loop that modifies the person's name 27 times.
for (int i = 0; i < 27; i++)
foreach (var person in persons)
person.Name = i.ToString();
await savePersonBatchTask;
The output will look like this
New batch arrived: 10
New batch arrived: 10
New batch arrived: 10
New batch arrived: 10
New batch arrived: 10
New batch arrived: 4
There is a lot more it can do and all that is explained very nicely on Introduction to Rx.
Review
And I also have a couple of comments about your code...
private readonly Queue<Tuple<string, object>> _saveOrders
You can simplify this and make it easier to use by using strong tuples with named properties. All you need is to throw away the Tuple
type and let the compiler derive the tuple for you.
private readonly Queue<(string FileName, object Item)> _saveOrders = ..;
later in code you also don't need it anymore
saveOrders.Enqueue((fileName,item));
Your while
loop will become simpler too because tuples can be deconstructed into variables:
while (_saveOrders.Count > 0)
var (fileName, item) = _saveOrders.Dequeue();
if (_saveOrders.All(o => o.FileName != fileName))
await SaveHelper.TrySaveToAppData(fileName, item);
add a comment |Â
up vote
1
down vote
accepted
up vote
1
down vote
accepted
Alternative: Rx
Creating your own producer/consumer solution is not always easy and is one option but you could also use the tools that are already there. One such tool are the Reactive Extensions (NuGet-Package: System.Reactive
). It can be very helpful in your scenario.
I requires two parties
- an observable and
- an observer
Your not yet real observables are the POCO objects that implement the INotifyPropertyChanged
interface. You can turn them into observables with helper factories that can derive an IObservable
from the PropertyChanged
event pattern.
class Person : INotifyPropertyChanged, IObservable<EventPattern<PropertyChangedEventArgs>>
private readonly IObservable<EventPattern<PropertyChangedEventArgs>> _observable;
private string _name;
public Person()
_observable =
Observable
.FromEventPattern<PropertyChangedEventHandler, PropertyChangedEventArgs>(
handler => PropertyChanged += handler,
handler => PropertyChanged -= handler
);
public event PropertyChangedEventHandler PropertyChanged;
public string Name
get => _name;
set
_name = value;
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Name)));
public IDisposable Subscribe(IObserver<EventPattern<PropertyChangedEventArgs>> observer)
return _observable.Subscribe(observer);
Now you only need a task to process a collection of them. You can create it in many different ways so here is just one of them. I created an observable from an array of persons that I Merge
into a single observable that I want to watch. I'd like this to be processed either every ten items or every two seconds (there are also other options). Then each batch is processed with the ForEachAsync
loop.
var persons = new
new Person Name = "John",
new Person Name = "Tom"
;
var savePersonBatchTask =
persons
.Merge()
.Buffer(timeSpan: TimeSpan.FromSeconds(2), count: 10)
.Where(batch => buffer.Count > 0)
.ForEachAsync(batch =>
Console.WriteLine($"New batch arrived: batch.Count");
);
You can test this simulation by creating another loop that modifies the person's name 27 times.
for (int i = 0; i < 27; i++)
foreach (var person in persons)
person.Name = i.ToString();
await savePersonBatchTask;
The output will look like this
New batch arrived: 10
New batch arrived: 10
New batch arrived: 10
New batch arrived: 10
New batch arrived: 10
New batch arrived: 4
There is a lot more it can do and all that is explained very nicely on Introduction to Rx.
Review
And I also have a couple of comments about your code...
private readonly Queue<Tuple<string, object>> _saveOrders
You can simplify this and make it easier to use by using strong tuples with named properties. All you need is to throw away the Tuple
type and let the compiler derive the tuple for you.
private readonly Queue<(string FileName, object Item)> _saveOrders = ..;
later in code you also don't need it anymore
saveOrders.Enqueue((fileName,item));
Your while
loop will become simpler too because tuples can be deconstructed into variables:
while (_saveOrders.Count > 0)
var (fileName, item) = _saveOrders.Dequeue();
if (_saveOrders.All(o => o.FileName != fileName))
await SaveHelper.TrySaveToAppData(fileName, item);
Alternative: Rx
Creating your own producer/consumer solution is not always easy and is one option but you could also use the tools that are already there. One such tool are the Reactive Extensions (NuGet-Package: System.Reactive
). It can be very helpful in your scenario.
I requires two parties
- an observable and
- an observer
Your not yet real observables are the POCO objects that implement the INotifyPropertyChanged
interface. You can turn them into observables with helper factories that can derive an IObservable
from the PropertyChanged
event pattern.
class Person : INotifyPropertyChanged, IObservable<EventPattern<PropertyChangedEventArgs>>
private readonly IObservable<EventPattern<PropertyChangedEventArgs>> _observable;
private string _name;
public Person()
_observable =
Observable
.FromEventPattern<PropertyChangedEventHandler, PropertyChangedEventArgs>(
handler => PropertyChanged += handler,
handler => PropertyChanged -= handler
);
public event PropertyChangedEventHandler PropertyChanged;
public string Name
get => _name;
set
_name = value;
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Name)));
public IDisposable Subscribe(IObserver<EventPattern<PropertyChangedEventArgs>> observer)
return _observable.Subscribe(observer);
Now you only need a task to process a collection of them. You can create it in many different ways so here is just one of them. I created an observable from an array of persons that I Merge
into a single observable that I want to watch. I'd like this to be processed either every ten items or every two seconds (there are also other options). Then each batch is processed with the ForEachAsync
loop.
var persons = new
new Person Name = "John",
new Person Name = "Tom"
;
var savePersonBatchTask =
persons
.Merge()
.Buffer(timeSpan: TimeSpan.FromSeconds(2), count: 10)
.Where(batch => buffer.Count > 0)
.ForEachAsync(batch =>
Console.WriteLine($"New batch arrived: batch.Count");
);
You can test this simulation by creating another loop that modifies the person's name 27 times.
for (int i = 0; i < 27; i++)
foreach (var person in persons)
person.Name = i.ToString();
await savePersonBatchTask;
The output will look like this
New batch arrived: 10
New batch arrived: 10
New batch arrived: 10
New batch arrived: 10
New batch arrived: 10
New batch arrived: 4
There is a lot more it can do and all that is explained very nicely on Introduction to Rx.
Review
And I also have a couple of comments about your code...
private readonly Queue<Tuple<string, object>> _saveOrders
You can simplify this and make it easier to use by using strong tuples with named properties. All you need is to throw away the Tuple
type and let the compiler derive the tuple for you.
private readonly Queue<(string FileName, object Item)> _saveOrders = ..;
later in code you also don't need it anymore
saveOrders.Enqueue((fileName,item));
Your while
loop will become simpler too because tuples can be deconstructed into variables:
while (_saveOrders.Count > 0)
var (fileName, item) = _saveOrders.Dequeue();
if (_saveOrders.All(o => o.FileName != fileName))
await SaveHelper.TrySaveToAppData(fileName, item);
edited Mar 30 at 9:49
answered Mar 30 at 9:32
t3chb0t
32.1k54195
32.1k54195
add a comment |Â
add a comment |Â
up vote
0
down vote
Problem I see is no call to force the save. You could have that in another method but you should not be repeating the save code.
Seems to me a producer consumer like BackGroundWorker would be a better approach here.
add a comment |Â
up vote
0
down vote
Problem I see is no call to force the save. You could have that in another method but you should not be repeating the save code.
Seems to me a producer consumer like BackGroundWorker would be a better approach here.
add a comment |Â
up vote
0
down vote
up vote
0
down vote
Problem I see is no call to force the save. You could have that in another method but you should not be repeating the save code.
Seems to me a producer consumer like BackGroundWorker would be a better approach here.
Problem I see is no call to force the save. You could have that in another method but you should not be repeating the save code.
Seems to me a producer consumer like BackGroundWorker would be a better approach here.
answered Feb 28 at 7:40
paparazzo
4,8131730
4,8131730
add a comment |Â
add a comment |Â
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f188458%2flimit-and-merge-saving-operations%23new-answer', 'question_page');
);
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password