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.