MVC.Net View Model

Question

I'm nearing the end of my first MVC project which uses VB.Net and MVC5. I've started writing all my view models in a very similar fashion. This has been working well for me, but being the first person on my team to use MVC, and fairly new to being a professional developer I'm curious if this is an acceptable way to write view models, and how I can improve upon this for future projects.

I start each model by defining a class that will hold all the properties for my view:

Public Class ItemHistoryPresentationModel
 Public Property UniqueItemID As Integer
 Public Property ItemTypeID As Integer
 Public Property ItemTypeString As String
 Public Property ListOfDatesChanged As List(Of Date)
 Public Property ListOfOperators As List(Of String)
 Public Property ListOfAssignedEmployee As List(Of String)
 Public Property ListOfChangeType As List(Of String)
 Public Property ListOfLocationIDs As List(Of Long)
 Public Property ListOfLocationNames As List(Of String)
 Public Property ListOfEdits As List(Of EditHistory)
 Public Property ListOfEditHistoryIDs As List(Of Integer?)
End Class

And then I create an object and populate the properties:

 Function getItemHistory(itemID As Integer, itemTypeID As Integer) As ItemHistoryPresentationModel
 Dim returnModel = New ItemHistoryPresentationModel
 returnModel.UniqueItemID = itemID
 returnModel.ItemTypeID = itemTypeID
 returnModel.ItemTypeString = getItemName(itemTypeID)
 returnModel.ListOfDatesChanged = getChangeDates(itemID)
 returnModel.ListOfOperators = getOperators(itemID)
 returnModel.ListOfAssignedEmployee = getAssignedEmployees(itemID)
 returnModel.ListOfChangeType = getChangeTypes(itemID)
 returnModel.ListOfLocationIDs = getLocationIDs(itemID)
 returnModel.ListOfLocationNames = getLocationNames(returnModel.ListOfLocationIDs)
 returnModel.ListOfEditHistoryIDs = getEditHistoryIDs(itemID)
 returnModel.ListOfEdits = getEditHistoryIDs(itemID, returnModel.ListOfEditHistoryIDs)
 Return returnModel
 End Function

And a brief example of one of the methods I use to populate a property:

 Function getChangeDates(itemID As Integer) As List(Of Date)
 Dim listOfDates = (From row In db.tblItem_History
 Where row.Unique_Item_ID = itemID
 Order By row.Item_History_ID
 Select row.Date_Of_Change).ToList()
 Return listOfDates
 End Function

I then pass the object to the view and use razor to extract and display the information.

I'm curious if this is a generally acceptable way of creating/using a view model, and if there is anything here that is terrible coding.

Answer 1

I'll address one specific aspect - performance.

Considering methods such as getChangeDates - they should return awaitable tasks (especially the ones involving asynchronous operations such as fetching from the database).

The flow of getItemHistory is completely synchronous and runs in O(n) time complexity. If each one of the nine getWhatever methods takes ~200 milliseconds to complete, the entire method method will take ~1800 milliseconds to complete. If all of those methods are run in parallel, the method will take only as long as the slowest running task (assuming you have n available threads).

Check out the following example in c#.

var getOperatorsTask = GetOperatorsAsync();
var getChangeTypesTask = GetChangeTypesAsync();
await Task.WhenAll(getOperatorsTask, getChangeTypesTask); // wait for all tasks to finish
returnModel.ListOfOperators = getOperatorsTask.Result;
returnModel.ListOfChangeType = getChangeTypesTask.Result;

Also, one suggestion regarding naming, don't use ListOfOperators. Simply Operators will suffice.