.net: how to optimize this code?

Please help me optimize/ refactor this code.....

private sub test
        Call PopulateColorsWithMasterIdentity(Colo开发者_JAVA百科rs, Id)
        Call PopulatePartsWithMasterIdentity(Parts, Id)
        Call PopulateSaloonsWithMasterIdentity(Saloons, Id)
End sub

Private Sub PopulateColorsWithMasterIdentity(ByRef MyList As List(Of entclsCriticalPartSetColor), ByVal Id As Integer)

    For index As Byte = 0 To MyList.Count - 1
        MyList.Item(index).CriticalPartsSetId = Id
    Next
End Sub

Private Sub PopulatePartsWithMasterIdentity(ByRef MyList As List(Of entclsCriticalPartSetPart), ByVal Id As Integer)

    For index As Byte = 0 To MyList.Count - 1
        MyList.Item(index).CriticalPartsSetId = Id
    Next
End Sub

Private Sub PopulateSaloonsWithMasterIdentity(ByRef MyList As List(Of entclsCriticalPartSetSaloon), ByVal Id As Integer)

    For index As Byte = 0 To MyList.Count - 1
        MyList.Item(index).CriticalPartsSetId = Id
    Next
End Sub

*EDIT*

Actually, Is it possible to use "Polymorphism"? I mean, instead of having 3 different parts of populateXXXWithMasterIdentity, can I have one PopulateListWithMasterIdentity like this one:

Private Sub PopulateListWithMasterIdentity(MyList As IList(Of entclsCriticalPartsBase), Id As Integer)
    .....
End Sub

Thank you

---

You can’t really optimize this code but you can make it a bit more concise because the Item property call is completely redundant.

Furthermore, please don’t use the Byte type for the index variable, that doesn’t make any sense, even if the lists never have more than 255 elements. Always use Integer here (or let the compiler infer your type by using Option Infer On).

Instead, write the loops as follows:

For index As Integer = 0 To MyList.Count - 1
    MyList(index).CriticalPartsSetId = Id
Next

---

First, declare an interface that contains the property CriticalPartsSetId which is common to all three types that you use:

Public Interface IHasCriticalPartsSetId
    Property CriticalPartsSetId As Integer
End Interface

Make sure that your three different types implement this interface.

Next, write one single method that updates the CriticalPartsSetId property:

Sub PopulateId(xs As IEnumerable(Of IHasCriticalPartsSetId), id As Integer)
    For Each x in xs
        x.CriticalPartsSetId = id
    Next
End Sub

Right now I'm not quite sure whether IEnumerable(Of IHasCriticalPartsSetId) will work as expected, or if you'll indeed need to use a generic type parameter constraint, as shown in Marc Gravell's answer — in VB.NET this would be: IEnumerable(Of T As IHasCriticalPartsSetId).

Note also another completely unrelated optimization: Getting rid of a For loop with a Byte counter. Using a For Each loop makes your code easier to read. (And using a Byte-typed counter variable might actually give you worse performance than using an Integer-typed variable, since (IIRC) the CLR expands Byte-sized values to Integer-sized values when it puts them on its internal execution stack.)

---

In the updates you are mutating every object; so it can't be better than O(n), which it is already. No optimisation is necessary. As a refactor, I'd take away the ByRef - you aren't using it. You might also consider a common base-class or interface, then all 3 could share a single generic method. In c# terms:

interface IFoo {
    int CriticalPartsSetId {get;set;}
}
public static void UpdateCriticalPartsSetId<T>(
    IEnumerable<T> items, int criticalPartsSetId)
    where T : IFoo
{
     foreach(var item in items)
        item.CriticalPartsSetId = criticalPartsSetId;
}

---

To answer your polymorphism question, yes you could. But you don't really need to. All three of these classes share a common property (CriticalPartsSetId), so you could put that on a base class and let all of them inherit it. Polymorphism would let you change something about that on the subclass, but in this case that isn't necessary since CriticalPartsSetId does the same thing.

I kind of wonder though, are the three lists you have here all parts of the same thing? Maybe it'd make more sense to do something like this:

Public Class CriticalPartsSet

    Public Property SetId as Integer
    Public Property SetColors as List(of entclsCriticalPartSetColor)
    Public Property SetParts as List(of entclsCriticalPartSetPart)
    Public Property SetSaloons as List(of entclsCriticalPartSetSaloon)

End Class

If you do that, the setId is part of the set itself and changing it is a single operation instead of an iteration on three lists. You can also pass around the entire set and everything about it as a single parameter to methods that need it, which is handy. :)

(Finally, as others as mentioned you should remove the ByRef and use ByVal. ByRef lets you replace the list with an entirely different list and have the caller get the new list, which is NOT the intended behaviour here. You can modify things inside the list with ByVal, and there's less chance for unintended oddness later.)