3 Replies - 906 Views - Last Post: 18 May 2013 - 08:01 AM

#1 depricated  Icon User is online

  • Behind Seven Proxies!

Reputation: 410
  • View blog
  • Posts: 1,423
  • Joined: 13-September 08

Critique my Code! Date QuickSort in VBA

Posted 15 May 2013 - 08:46 AM

I don't believe the language is the important part of this , thus why I'm not posting it in a language-specific forum. While specific languages will have ways of handling specifics tasks better, the point of this is for me to learn about better coding in general. I'm noob, you're all amazing, and I'd love to learn by having this picked apart and reconstructed.

The structure for the QuickSort is what was taught me by one of my professors, adapted for the specific situation for which it's being used - to sort an ActiveX Listview Box according to dates, and move empty dates to the end of the list.

Public Sub SortByDate(ByRef lvwBox As Object, dateColumn As Integer, left As Integer, right As Integer)
    
    Dim rightIndex As Integer
    Dim leftIndex As Integer
    Dim comparisonItem As Variant
    Dim storedItem As ListItem
    

    leftIndex = left
    rightIndex = right
    comparisonItem = lvwBox.ListItems(CInt((leftIndex + rightIndex) / 2)).SubItems(dateColumn)

    Do
        If SafeInsert(lvwBox.ListItems(leftIndex).SubItems(dateColumn)) = "" Then
            Do While (CDate(Date) > CDate(comparisonItem) And leftIndex < right)
                leftIndex = leftIndex + 1
            Loop
        Else
            Do While (lvwBox.ListItems(leftIndex).SubItems(dateColumn) < CDate(comparisonItem)) And leftIndex < right
                leftIndex = leftIndex + 1
            Loop
        End If
            
        If SafeInsert(lvwBox.ListItems(rightIndex).SubItems(dateColumn)) = "" Then
            Do While (CDate(comparisonItem) > CDate(Date) And rightIndex > left)
                rightIndex = rightIndex - 1
            Loop
        Else
            Do While (CDate(comparisonItem) < (lvwBox.ListItems(rightIndex).SubItems(dateColumn)) And rightIndex > left)
                rightIndex = rightIndex - 1
            Loop
        End If
        
        If leftIndex <= rightIndex Then
            Dim i As Integer
            Dim movedItem As ListItem
            Set storedItem = lvwBox.ListItems(leftIndex)
            
            lvwBox.ListItems.Remove leftIndex
            Set movedItem = lvwBox.ListItems.Add(leftIndex)
            
            movedItem.text = lvwBox.ListItems(rightIndex)
            For i = 1 To lvwBox.ListItems(rightIndex).ListSubItems.Count
                movedItem.SubItems(i) = SafeInsert(lvwBox.ListItems(rightIndex).ListSubItems(i))
            Next
            
            lvwBox.ListItems(rightIndex) = SafeInsert(storedItem)
            For i = 1 To storedItem.ListSubItems.Count
                lvwBox.ListItems(rightIndex).SubItems(i) = storedItem.ListSubItems(i)
            Next
            
            leftIndex = leftIndex + 1
            rightIndex = rightIndex - 1
        End If
    Loop While leftIndex <= rightIndex
    
    If left < rightIndex Then
        SortByDate lvwBox, dateColumn, left, rightIndex
    End If
    
    If leftIndex < right Then
        SortByDate lvwBox, dateColumn, leftIndex, right
    End If
    
End Sub


Is This A Good Question/Topic? 0
  • +

Replies To: Critique my Code! Date QuickSort in VBA

#2 stackoverflow  Icon User is offline

  • D.I.C Addict
  • member icon

Reputation: 164
  • View blog
  • Posts: 545
  • Joined: 06-July 11

Re: Critique my Code! Date QuickSort in VBA

Posted 15 May 2013 - 11:10 AM

-1
Was This Post Helpful? 0
  • +
  • -

#3 Skydiver  Icon User is offline

  • Code herder
  • member icon

Reputation: 3161
  • View blog
  • Posts: 9,541
  • Joined: 05-May 12

Re: Critique my Code! Date QuickSort in VBA

Posted 16 May 2013 - 02:39 PM

I agree. -1.

That is terrible code because of:
What the heck is SafeInsert() and why am I calling it as part of the condition for if statements? Is it this one?
No BeginUpdate() and EndUpdate() calls to prevent repainting while the items are being sorted.
No option to sort by ascending vs descending.
The tail recursion on lines 61-63 can be removed and the code transformed to be partly recursive and partly iterative.
The constant dereferencing of ListView.Items[row].SubItems[col] is slow, and verbose.
The swapping of rows on lines 36-60 is overly complex.
The swapping of rows on lines 36-60 fails to copy over other ListViewItem properties like Tag, colors, selection state, etc.
The swapping of rows on lines 36-60 transforms nulls into empty strings assuming SafeInsert() is the same as the one here.

I'm wondering why you aren't using the ListView.Sort() and ListView.ListViewItemSorter. Is it because these are not present in the VBA version of ListView?
http://msdn.microsof...itemsorter.aspx
Was This Post Helpful? 0
  • +
  • -

#4 BobRodes  Icon User is offline

  • Your Friendly Local Curmudgeon
  • member icon

Reputation: 571
  • View blog
  • Posts: 2,979
  • Joined: 19-May 09

Re: Critique my Code! Date QuickSort in VBA

Posted 18 May 2013 - 08:01 AM

This might just be an academic exercise to learn the mechanics of sorting. If so, using the Sort method doesn't apply. Depricated, without going into the logic of your sort algorithm (I fell on my head as a child, and am naturally lazy to boot), I can say that it would be better to first copy your values to an array, sort the array into another array, empty your listview, and then repopulate it from the array. Calls to objects are much slower than calls to array elements. That said, it would also be interesting to see if you could do it quicker by availing yourself of the List object's Insert method. Also, this is a good overview of various sorting algorithms.

This post has been edited by BobRodes: 18 May 2013 - 08:09 AM

Was This Post Helpful? 0
  • +
  • -

Page 1 of 1