Skip to main content
Mention LastIndexOf is inefficient
Source Link
Mark Hurd
  • 346
  • 3
  • 9
  • Your implementation of Sort would be optimised by implementing IndexOfMin so that Min = Item(IndexOfMin) (with a caveat for an empty List), but you can then use the O(1) RemoveAt instead of the O(n) Remove at the end of the Do Until loop. Similarly for IndexOfMax, Max and SortDescending, of course. I'd consider making IndexOfMin and IndexOfMax Public to allow other code to use the same optimisation.

  • IMHO, especially from a VB6 POV and contrary to Mat's Mug, sorting an empty List should succeed and return the empty List, not throw an error. I.e. IsSortable should start:

      IsSortable = True
      If Count = 0 Then Exit Function
    

Your existing Sort and SortDescending implementations don't need to change for this.

  • (EDIT: Added 6 months later :-) ) Your implementation of LastIndexOf should actually be the same as IndexOf, but with For i = Count To 1 Step -1.

Minor quibbles

  • This is more readable (from a VB6 POV, anyway), and, of course, minusculely faster:

      Public Sub RemoveRange(ByVal Index As Long, ByVal valuesCount As Long)
      'Removes a range of elements from the List.
    
          Dim i As Long
          For i = 1 To valuesCount
    
              RemoveAt Index
    
          Next
    
      End Sub
    
  • value is unused in AddValues.

  • Clear should use RemoveAt 1. (I've forgotten my VB6 optimisations: I know RemoveAt 1 is the standard idiom, but is RemoveAt Count faster? NO: I've checked it and RemoveAt 1 is 9 orders of magnitude faster!)

  • Your implementation of Sort would be optimised by implementing IndexOfMin so that Min = Item(IndexOfMin) (with a caveat for an empty List), but you can then use the O(1) RemoveAt instead of the O(n) Remove at the end of the Do Until loop. Similarly for IndexOfMax, Max and SortDescending, of course. I'd consider making IndexOfMin and IndexOfMax Public to allow other code to use the same optimisation.

  • IMHO, especially from a VB6 POV and contrary to Mat's Mug, sorting an empty List should succeed and return the empty List, not throw an error. I.e. IsSortable should start:

      IsSortable = True
      If Count = 0 Then Exit Function
    

Your existing Sort and SortDescending implementations don't need to change for this.

Minor quibbles

  • This is more readable (from a VB6 POV, anyway), and, of course, minusculely faster:

      Public Sub RemoveRange(ByVal Index As Long, ByVal valuesCount As Long)
      'Removes a range of elements from the List.
    
          Dim i As Long
          For i = 1 To valuesCount
    
              RemoveAt Index
    
          Next
    
      End Sub
    
  • value is unused in AddValues.

  • Clear should use RemoveAt 1. (I've forgotten my VB6 optimisations: I know RemoveAt 1 is the standard idiom, but is RemoveAt Count faster? NO: I've checked it and RemoveAt 1 is 9 orders of magnitude faster!)

  • Your implementation of Sort would be optimised by implementing IndexOfMin so that Min = Item(IndexOfMin) (with a caveat for an empty List), but you can then use the O(1) RemoveAt instead of the O(n) Remove at the end of the Do Until loop. Similarly for IndexOfMax, Max and SortDescending, of course. I'd consider making IndexOfMin and IndexOfMax Public to allow other code to use the same optimisation.

  • IMHO, especially from a VB6 POV and contrary to Mat's Mug, sorting an empty List should succeed and return the empty List, not throw an error. I.e. IsSortable should start:

      IsSortable = True
      If Count = 0 Then Exit Function
    

Your existing Sort and SortDescending implementations don't need to change for this.

  • (EDIT: Added 6 months later :-) ) Your implementation of LastIndexOf should actually be the same as IndexOf, but with For i = Count To 1 Step -1.

Minor quibbles

  • This is more readable (from a VB6 POV, anyway), and, of course, minusculely faster:

      Public Sub RemoveRange(ByVal Index As Long, ByVal valuesCount As Long)
      'Removes a range of elements from the List.
    
          Dim i As Long
          For i = 1 To valuesCount
    
              RemoveAt Index
    
          Next
    
      End Sub
    
  • value is unused in AddValues.

  • Clear should use RemoveAt 1. (I've forgotten my VB6 optimisations: I know RemoveAt 1 is the standard idiom, but is RemoveAt Count faster? NO: I've checked it and RemoveAt 1 is 9 orders of magnitude faster!)

Benchmarked RemoveAt 1 and RemoveAt Count
Source Link
Mark Hurd
  • 346
  • 3
  • 9
  • Your implementation of Sort would be optimised by implementing IndexOfMin so that Min = Item(IndexOfMin) (with a caveat for an empty List), but you can then use the O(1) RemoveAt instead of the O(n) Remove at the end of the Do Until loop. Similarly for IndexOfMax, Max and SortDescending, of course. I'd consider making IndexOfMin and IndexOfMax Public to allow other code to use the same optimisation.

  • IMHO, especially from a VB6 POV and contrary to Mat's Mug, sorting an empty List should succeed and return the empty List, not throw an error. I.e. IsSortable should start:

      IsSortable = True
      If Count = 0 Then Exit Function
    

Your existing Sort and SortDescending implementations don't need to change for this.

Minor quibbles

  • This is more readable (from a VB6 POV, anyway), and, of course, minusculely faster:

      Public Sub RemoveRange(ByVal Index As Long, ByVal valuesCount As Long)
      'Removes a range of elements from the List.
    
          Dim i As Long
          For i = 1 To valuesCount
    
              RemoveAt Index
    
          Next
    
      End Sub
    
  • value is unused in AddValues.

  • Clear should use RemoveAt 1. (I've forgotten my VB6 optimisations:I've forgotten my VB6 optimisations: I know RemoveAt 1 is the standard idiom, but is RemoveAt Count faster? NO: I've checked it and RemoveAt 1 is 9 orders of magnitude faster!)

  • Your implementation of Sort would be optimised by implementing IndexOfMin so that Min = Item(IndexOfMin) (with a caveat for an empty List), but you can then use the O(1) RemoveAt instead of the O(n) Remove at the end of the Do Until loop. Similarly for IndexOfMax, Max and SortDescending, of course. I'd consider making IndexOfMin and IndexOfMax Public to allow other code to use the same optimisation.

  • IMHO, especially from a VB6 POV and contrary to Mat's Mug, sorting an empty List should succeed and return the empty List, not throw an error. I.e. IsSortable should start:

      IsSortable = True
      If Count = 0 Then Exit Function
    

Your existing Sort and SortDescending implementations don't need to change for this.

Minor quibbles

  • This is more readable (from a VB6 POV, anyway), and, of course, minusculely faster:

      Public Sub RemoveRange(ByVal Index As Long, ByVal valuesCount As Long)
      'Removes a range of elements from the List.
    
          Dim i As Long
          For i = 1 To valuesCount
    
              RemoveAt Index
    
          Next
    
      End Sub
    
  • value is unused in AddValues.

  • Clear should use RemoveAt 1. (I've forgotten my VB6 optimisations: I know RemoveAt 1 is the standard idiom, but is RemoveAt Count faster?)

  • Your implementation of Sort would be optimised by implementing IndexOfMin so that Min = Item(IndexOfMin) (with a caveat for an empty List), but you can then use the O(1) RemoveAt instead of the O(n) Remove at the end of the Do Until loop. Similarly for IndexOfMax, Max and SortDescending, of course. I'd consider making IndexOfMin and IndexOfMax Public to allow other code to use the same optimisation.

  • IMHO, especially from a VB6 POV and contrary to Mat's Mug, sorting an empty List should succeed and return the empty List, not throw an error. I.e. IsSortable should start:

      IsSortable = True
      If Count = 0 Then Exit Function
    

Your existing Sort and SortDescending implementations don't need to change for this.

Minor quibbles

  • This is more readable (from a VB6 POV, anyway), and, of course, minusculely faster:

      Public Sub RemoveRange(ByVal Index As Long, ByVal valuesCount As Long)
      'Removes a range of elements from the List.
    
          Dim i As Long
          For i = 1 To valuesCount
    
              RemoveAt Index
    
          Next
    
      End Sub
    
  • value is unused in AddValues.

  • Clear should use RemoveAt 1. (I've forgotten my VB6 optimisations: I know RemoveAt 1 is the standard idiom, but is RemoveAt Count faster? NO: I've checked it and RemoveAt 1 is 9 orders of magnitude faster!)

Source Link
Mark Hurd
  • 346
  • 3
  • 9

  • Your implementation of Sort would be optimised by implementing IndexOfMin so that Min = Item(IndexOfMin) (with a caveat for an empty List), but you can then use the O(1) RemoveAt instead of the O(n) Remove at the end of the Do Until loop. Similarly for IndexOfMax, Max and SortDescending, of course. I'd consider making IndexOfMin and IndexOfMax Public to allow other code to use the same optimisation.

  • IMHO, especially from a VB6 POV and contrary to Mat's Mug, sorting an empty List should succeed and return the empty List, not throw an error. I.e. IsSortable should start:

      IsSortable = True
      If Count = 0 Then Exit Function
    

Your existing Sort and SortDescending implementations don't need to change for this.

Minor quibbles

  • This is more readable (from a VB6 POV, anyway), and, of course, minusculely faster:

      Public Sub RemoveRange(ByVal Index As Long, ByVal valuesCount As Long)
      'Removes a range of elements from the List.
    
          Dim i As Long
          For i = 1 To valuesCount
    
              RemoveAt Index
    
          Next
    
      End Sub
    
  • value is unused in AddValues.

  • Clear should use RemoveAt 1. (I've forgotten my VB6 optimisations: I know RemoveAt 1 is the standard idiom, but is RemoveAt Count faster?)