2
\$\begingroup\$

I worried about this code in a ViewModel. ViewModel should not contain any code except binding's. But without async update the ui would freeze. How do I improve it? I'm restricted to using .net 3.5 or below.

public ShippingDocumentsRegisterViewModel()
{
    this.Columns = model.InitializeColumns();
    Action<object> Documents = new Action<object>(GetDocumentsAsync);
    IAsyncResult result = Documents.BeginInvoke(new AsyncCallback(GetDocumentsAsync),null, null);
}

public void GetDocumentsAsync(object param)
{
    this.ShippingDocuments = model.GetDocuments();
}

Model:

class ShippingDocumentsModel
    {
        public ObservableCollection<ShippingDocument> GetDocuments()
        {
            var documents = new ObservableCollection<ShippingDocument>();
            for (int i = 0; i < 100; i++)
            {
                System.Threading.Thread.Sleep(50);
                documents.Add(new ShippingDocument { Name = i.ToString() });
            }
            return documents;
        }

        public ObservableCollection<ShippingDocumentColumDescriptor> InitializeColumns()
        {
            return new ObservableCollection<ShippingDocumentColumDescriptor>
            {
                new ShippingDocumentColumDescriptor { HeaderText = "Статус", DisplayMember = "Status" },
                new ShippingDocumentColumDescriptor { HeaderText = "Підпис", DisplayMember = "Signature" }
            };
        }
    }

ViewModel:

public class ShippingDocumentsRegisterViewModel : ViewModelBase
    {
        ShippingDocumentsModel model = new ShippingDocumentsModel();

        public ShippingDocumentsRegisterViewModel()
        {
            this.Columns = model.InitializeColumns();
            Action<object> Documents = new Action<object>(GetDocumentsAsync);
            IAsyncResult result = Documents.BeginInvoke(new AsyncCallback(GetDocumentsAsync),null, null);
        }

        public void GetDocumentsAsync(object param)
        {
            this.ShippingDocuments = model.GetDocuments();
        }

        private ObservableCollection<ShippingDocument> shippingDocuments;

        public ObservableCollection<ShippingDocument> ShippingDocuments
        {
            get
            {
                return shippingDocuments;
            }

            private set
            {
                shippingDocuments = value;
                RaisePropertyChanged("ShippingDocuments");
            }
        }

        public ObservableCollection<ShippingDocumentColumDescriptor> Columns { get; private set; }

        private ICommand _addColumnCommand;

        public ICommand AddColumnCommand
        {
            get
            {
                if (_addColumnCommand == null)
                {
                    _addColumnCommand = new RelayCommand<string>(
                        s =>
                        {
                            this.Columns.Add(new ShippingDocumentColumDescriptor { HeaderText = s, DisplayMember = s });
                        });
                }
                return _addColumnCommand;
            }
        }

        private ICommand _removeColumnCommand;

        public ICommand RemoveColumnCommand
        {
            get
            {
                if (_removeColumnCommand == null)
                {
                    _removeColumnCommand = new RelayCommand<string>(
                        s =>
                        {
                            this.Columns.Remove(this.Columns.FirstOrDefault(d => d.DisplayMember == s));
                        });
                }
                return _removeColumnCommand;
            }
        }
    }

Edit:

I think maybe this solution would be better?

BackgroundWorker BW = new BackgroundWorker();
            BW.DoWork += (o, e) => 
            { 
                this.ShippingDocuments = model.GetDocuments(); 
            };
            BW.RunWorkerAsync();
\$\endgroup\$
  • 3
    \$\begingroup\$ What is the System.Threading.Thread.Sleep(50); in the model for? \$\endgroup\$ – JanDotNet Jul 25 '16 at 12:33
7
\$\begingroup\$

General

  • Your model should not return an ObservableCollection because that collection should be used only if its change notification ability is needed (e.g. with data binding). The method GetDocuments should return an Array or an IEnumerable.
  • The method InitializeColumns also seems to be GUI related. Consider moving it to the view model.

Background Processing

If you have to load documents in the background, I would suggest using BackgroundWorker (for .Net Framework 3.5 and below). But you should use it more like:

  • Pass the model to the RunWorkerAsyncmethod and use the Argument property to access it.
  • Do not access view model properties from within the DoWork delegate because that is the code that will be executed in the background. Assign the calculated result to the Result property instead.
  • Use the RunWorkCompleted method to process the calculated result on the GUI thread.

e.g:

var bw = new BackgroundWorker();
bw.DoWork += (o, e) => e.Result = (e.Argument as Model).GetDocuments();
bw.RunWorkerCompleted += (s, e) => ShippingDocuments = new ObservableCollection((ShippingDocument[])e.Result);
bw.RunWorkerAsync(model);
\$\endgroup\$
  • 2
    \$\begingroup\$ ObservableCollection is not GUI related itself. It just so happens, that it is used by binding engine. But it does not mean, that you can't use it in your model or business layer, if you need to watch for collection changes. In this case however ShippingDocumentsModel looks like a viewmodel to me, and I agree that this logic should probably moved to ShippingDocumentsRegisterViewModel. OP seem to have this misconception, that ViewModel shoud not contain any code exept binding's, which is nonsense. \$\endgroup\$ – Nikita B Jul 25 '16 at 13:53
  • \$\begingroup\$ ObservableCollection is not GUI related itself. That is right - I removed that part from the anwer. Thanks. \$\endgroup\$ – JanDotNet Jul 25 '16 at 14:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.