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();
1 Answer 1
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 methodGetDocuments
should return anArray
or anIEnumerable
. - 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
RunWorkerAsync
method and use theArgument
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 theResult
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);
-
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 howeverShippingDocumentsModel
looks like a viewmodel to me, and I agree that this logic should probably moved toShippingDocumentsRegisterViewModel
. OP seem to have this misconception, thatViewModel shoud not contain any code exept binding's
, which is nonsense. \$\endgroup\$Nikita B– Nikita B2016年07月25日 13:53:51 +00:00Commented Jul 25, 2016 at 13:53 -
\$\begingroup\$
ObservableCollection
is not GUI related itself. That is right - I removed that part from the anwer. Thanks. \$\endgroup\$JanDotNet– JanDotNet2016年07月25日 14:01:23 +00:00Commented Jul 25, 2016 at 14:01
System.Threading.Thread.Sleep(50);
in the model for? \$\endgroup\$