You shouldn't do calculations or any time consuming operations in the getter or setter of a property. This can drastically degrade performance. If the calculations or operation is time consuming, then you should execute it in a background thread and raise the PropertyChanged
event once the Task
has completed. This way the invocation of the getter or setter of a property won't freeze the UI.
Explanation of the behavior you observed:
The side effect of changing the property value in its own a getter instead of the setter is that the new value won't propagate to the binding targets. The getter is only invoked by a binding when the PropertyChanged
event occurred. So doing the calculations in the getter doesn't trigger the binding to refresh. Now when reloading the page, all bindings will initialize the binding target and therefore invoke the property getter.
You have to set the TotalPrice
property (and not the backing field) in order to trigger a refresh of the binding target. But as you already experienced yourself, raising the PropertyChanged
event of a property in the
same getter will lead to an infinite loop and therefore to a StackOverflowException
.
Also the calculation will be always executed whenever the property's getter is accessed - even when the TotalPrice
hasn't changed.
The value of TotalPrice
depends on Products
property. To minimize the occurrence of the calculation of TotalPrice
, only calculate when Products
has changed:
OrderViewModel.cs
public class OrderViewModel : ViewModelBase
{
private decimal _totalPrice;
public decimal TotalPrice
{
get => this._totalPrice;
set
{
if (this._totalPrice == value)
return;
this._totalPrice = value;
RaisePropertyChanged();
}
}
private ObservableCollection<Product> _products;
public ObservableCollection<Product> Products
{
get => this._products;
set
{
if (this.Products == value)
return;
if (this.Products != null)
{
this.Products.CollectionChanged -= OnCollectionChanged;
UnsubscribeFromItemsPropertyChanged(this.Products);
}
this._products = value;
this.Products.CollectionChanged += OnCollectionChanged;
if (this.Products.Any())
{
SubscribeToItemsPropertyChanged(this.Products);
}
RaisePropertyChanged();
}
}
private void OnCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
{
if (!e.Action.Equals(NotifyCollectionChangedAction.Move))
{
UnsubscribeFromItemsPropertyChanged(e.OldItems);
SubscribeToItemsPropertyChanged(e.NewItems);
}
CalculateTotalPrice();
}
private void ProductChanged(object sender, PropertyChangedEventArgs e) => CalculateTotalPrice();
private void SubscribeToItemsPropertyChanged(IList newItems) => newItems?.OfType<INotifyPropertyChanged>().ToList().ForEach((item => item.PropertyChanged += ProductChanged));
private void UnsubscribeFromItemsPropertyChanged(IEnumerable oldItems) => oldItems?.OfType<INotifyPropertyChanged>().ToList().ForEach((item => item.PropertyChanged -= ProductChanged));
private void CalculateTotalPrice() => this.TotalPrice = this.Products.Sum(item => item.total_price);
private void GetProducts()
{
using (var context = new mainEntities())
{
var result = context.product.Include(c => c.brand);
this.Products = new ObservableCollection<Product>(
result.Select(item => new Product(item.name, item.mass, item.ean, item.brand.name, item.price)));
}
}
public void ResetOrder()
{
this.Products
.ToList()
.ForEach(product => product.Reset());
this.TotalPrice = 0;
}
public OrderViewModel()
{
SetView("Dodaj zamówienie");
GetProducts();
}
}
Also make sure that Product
(the items in the Products
collection) implements INotifyPropertyChanged
too. This will ensure that Products.CollectionChanged
event is raised when Product
properties have changed.
To fix the page switching behavior you have to modify the MainViewModel
class:
MainViewModel.cs
public class MainViewModel : ViewModelBase
{
// The page viewmodels
private Dictionary<string, ViewModelBase> PageViewModels { get; set; }
public Stack<string> ViewsQueue;
public MainViewModel()
{
User = new User(1, "login", "name", "surname", 1, 1, 1);
this.PageViewModels = new Dictionary<string, ViewModelBase>()
{
{"login", new LoginViewModel()},
{"menu", new MenuViewModel()},
{"order", new OrderViewModel()},
{"clients", new ClientsViewModel(User)}
};
this.CurrentViewModel = this.PageViewModels["login"];
this.ViewsQueue = new Stack<string>();
this.ViewsQueue.Push("login");
Messenger.Default.Register<NavigateTo>(
this,
(message) =>
{
try
{
ViewsQueue.Push(message.Name);
if (message.user != null) User = message.user;
SwitchView(message.Name);
}
catch (System.InvalidOperationException e)
{
}
});
Messenger.Default.Register<GoBack>(
this,
(message) =>
{
try
{
ViewsQueue.Pop();
SwitchView(ViewsQueue.Peek());
}
catch (System.InvalidOperationException e)
{
}
});
}
public RelayCommand<string> GoTo => new RelayCommand<string>(
viewName =>
{
ViewsQueue.Push(viewName);
SwitchView(viewName);
});
protected void SwitchView(string name)
{
if (this.PageViewModels.TryGetValue(name, out ViewModelBase nextPageViewModel))
{
if (nextPageViewModel is OrderViewModel orderViewModel)
orderViewModel.ResetOrder();
this.CurrentViewModel = nextPageViewModel;
}
}
}
Your modifified Product.cs
public class Product : ViewModelBase
{
public long id { get; set; }
public string name { get; set; }
public decimal mass { get; set; }
public long ean { get; set; }
public long brand_id { get; set; }
public string img_source { get; set; }
public string brand_name { get; set; }
private decimal _price;
public decimal price
{
get => this._price;
set
{
if (this._price == value)
return;
this._price = value;
OnPriceChanged();
RaisePropertyChanged();
}
}
private long _amount;
public long amount
{
get => this._amount;
set
{
if (this._amount == value)
return;
this._amount = value;
OnAmountChanged();
RaisePropertyChanged();
}
}
private decimal _total_price;
public decimal total_price
{
get => this._total_price;
set
{
if (this._total_price == value)
return;
this._total_price = value;
RaisePropertyChanged();
}
}
public Product(long id, string name, decimal mass, long ean, long brandId, decimal price, string imgSource)
{
this.id = id;
this.name = name;
this.mass = mass;
this.ean = ean;
this.brand_id = brandId;
this.price = price;
this.img_source = imgSource;
}
public Product(string name, decimal mass, long ean, string brandName, decimal price)
{
this.id = this.id;
this.name = name;
this.mass = mass;
this.ean = ean;
this.brand_name = brandName;
this.price = price;
}
public void Reset()
{
// Resetting the `amount` will trigger recalculation of `total_price`
this.amount = 0;
}
protected virtual void OnAmountChanged()
{
CalculateTotalPrice();
}
private void OnPriceChanged()
{
CalculateTotalPrice();
}
private void CalculateTotalPrice()
{
this.total_price = this.price * this.amount;
}
}
The issue was that you have always created a new view model when switching to a page. Of course all previous page information gets lost. You have to reuse the same view model instance. To do this, just store them in a dedicated private property that you initialize once in the constructor.