Many similar ViewModel methods to fetch and prepare model data

I have a MVVM application that fetches data from active directory and displays it in a DataGrid. The exact data displayed is determined by which query a user runs, and there are multiple available queries. Each query requires slightly different logic, but they all have the same general idea. These methods are all in my ViewModel. Data is a property that is bound to the DataGrid in the View.

If you would like to see any additional code for this review, please let me know. I am attempting to divide this project (several thousand lines long) into reasonably sized chunks for review. These methods are contributing greatly to the length, especially considering I intend on adding even more query options in the future. As of now, the whole class is 1000 lines long, with these methods being close to 400 lines.

```
private async void GetComputerGroupsCommandExecute()
{
StartTask();
QueryType = QueryType.ContextualComputerGroups;
await Task.Run(() =>
{
var computerPrincipal = ComputerPrincipal.FindByIdentity(
_principalContext, GetSelectedComputerDistinguishedName());
_dataPreparer = new DataPreparer
{
Data = new[]
{
ActiveDirectorySearcher.GetComputerGroups(
computerPrincipal)
},
Attributes = DefaultComputerGroupsAttributes.ToList()
};
try
{
Data = new List(_dataPreparer.GetResults())
.ToDataTable().AsDataView();
}
catch (ArgumentNullException)
{
ShowMessage(
"No groups found for the selected computer.");
}
});
FinishTask();
}

private async void GetDirectReportDirectReportsCommandExecute()
{
StartTask();
QueryType = QueryType.ContextualDirectReportDirectReports;
await Task.Run(() =>
{
var directReportUserPrincipal = UserPrincipal.FindByIdentity(
_principalContext,

• The methods should return Task instead of void. Otherwise exception are shallowed. Alternative you could encapsulate the whole content of the method in a try catch block.

See also: Asynchrone return types - Void Return Type

• Try to follow the DRY principle.

For example the line:

new List(_dataPreparer.GetResults()).ToDataTable().AsDataView();

may be extracted to a method:

private static IList GetData(DataPreparer dataPreparer) { ... }

• It seems that the instance variables _dataPreparer, _activeDirectorySearcher class variables are set / used from different methods. That increases the comlexity of the program flow. I would try to avoid such 'global writable states'.

The view model seems to be used as universal container with high coupling. In the medium term, I would try to move the logic of each method to a single class that is independent of the view model.

StackExchange Code Review Q#133860, answer score: 2