0

I'm working on project with MVVM Light framework.
I have MainViewModel which helps me to navigate between viewmodels. I have GoBack and GoTo methods. Their are changing CurrentViewModel.

private RelayCommand<string> _goTo;
public RelayCommand<string> GoTo
        {
            get
            {
                return _goTo
                    ?? (_goTo = new RelayCommand<string>(view
                     =>
                    {
                        SwitchView(view);
                    }));
            }
        }
 private void SwitchView(string name)
        {
            switch (name)
            {
                case "login":
                    User = null;
                    CurrentViewModel = new LoginViewModel();
                    break;
                case "menu":
                    CurrentViewModel = new MenuViewModel();
                    break;
                case "order":
                    CurrentViewModel = new OrderViewModel();
                    break;
            }

In MainWindow there is content control and data templates.

[...]
 <DataTemplate DataType="{x:Type vm:LoginViewModel}">
            <view:Login/>
        </DataTemplate>
        <DataTemplate DataType="{x:Type vm:MenuViewModel}">
            <view:Menu/>
        </DataTemplate>
[...]

<ContentControl VerticalAlignment="Top" HorizontalAlignment="Stretch" 
                            Content="{Binding CurrentViewModel}" IsTabStop="false"/>

In my OrderView (it is UserControl) I have textblock which should shows TotalPrice of order.

<TextBlock Text="{Binding AddOrderView.TotalPrice}"  Padding="0 2 0 0" FontSize="20" FontWeight="Bold" HorizontalAlignment="Right"/>

OrderViewModel has a property TotalPrice and it works well. When I am debugging I see that it has been changed, but in my View nothing happend.

        private decimal _totalPrice;
        public decimal TotalPrice
        {
            get
            {
                _totalPrice = 0;
                foreach (var item in Products)
                {
                    item.total_price = item.amount * item.price;
                    _totalPrice += item.price * item.amount;
                }
                return _totalPrice;
            }
            set
            {
                if (_totalPrice == value)
                    return;
                _totalPrice = value;
                RaisePropertyChanged("TotalPrice");
            }
        }

OrderViewModel iherits from BaseViewModel and it implements INotifyPropertyChanged.

Why is my textblock not updating/refreshing? How to do this?

When I change view with back button and go again to OrderView I see changes!

I spend few days to looking for solution and nothing helps me.

https://i.stack.imgur.com/K8lip.gif

So it's look like when View is seting there is no way to change it without reload its. I don't know how it works exactly.

2 Answers2

0

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 PropertyChangedevent once the Taskhas 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.

BionicCode
  • 1
  • 4
  • 28
  • 44
-1

It's not updating because you're only calling RaisePropertyChanged("TotalPrice"); in the setter. Whereas in your getter is the calculation. So anytime you change the Products property or the content of the Products collection, you also need to call RaisePropertyChanged("TotalPrice"); to notify the View that TotalPrice has been updated.

So if you change any of the item.amount or item.price or if you add or remove items from the Products list then you need to also call. RaisePropertyChanged("TotalPrice");

Ex:

Products.Add(item);
RaisePropertyChanged("TotalPrice"); //This will tell you're View to check for the new value from TotalPrice
BionicCode
  • 1
  • 4
  • 28
  • 44
Ben Walton
  • 399
  • 3
  • 11
  • When I added RaisePropertyChanged("TotalPrice"); to getter it evokes StackOverflowException. I added it below foreach loop. Also I can't call RaisePropertyChanged("TotalPrice") when I am changing amount becouse I have used NumericUpDown control and it's does't support command event. – Damian Hoop Jul 10 '19 at 19:05
  • Yes this will trigger an infinite loop. `RaisePropertyChanged` will raise the event which triigers the binding to call the getter. The getter again raises `RaisePropertyChanged` which triggers the binding to call the getter and so on – BionicCode Jul 10 '19 at 19:09
  • Do not add to the getter. Add to you're viewmodel where you set the other properties, or the setters of the other properties (if they are within scope, same class ), Not sure why you down voted me this is the correct answer. – Ben Walton Jul 10 '19 at 19:41
  • Basically what I'm saying is there are other properties that TotalPrice depend on, so when those other properties change, you need to call RaisePropertyChanged("TotalPrice") to notify the UI to recall the getter and recalculate TotalPrice and re display it. – Ben Walton Jul 10 '19 at 19:51
  • "So anytime you change a component in the getter you also need to call RaisePropertyChanged("TotalPrice"); to notify the View that Total Price has been updated.". This reads like: call `PropertyChanged` from the getter that does the calculations. I think instead of raising the event you should recalculate the new value of `TotalPrice` whem the `Products` collection chaged. – BionicCode Jul 10 '19 at 19:52
  • The point is, your implementation recalculates the `TotalPrice` everytime an item is added to `Products` (which is right) but also everytime the getter of `TotalPrice` is invoked. It will recalculate even when `Products` hasn't changed. – BionicCode Jul 10 '19 at 19:54
  • That's why you shouldn't do anything in the getter of a property in general. – BionicCode Jul 10 '19 at 19:55