CODEWITHSUNDEEP
excelvbaperformance
Max Segal
Max Segal

Reputation: 2045

VBA code efficiency/potential memory leak?

I have the following code in VBA which flips hebrew letters in each word in excel (omits english numbers and the rest:

Sub reverseHebrewInSelection()
   Application.Calculation = xlManual
   Dim c As Range
   For Each c In Selection
       c.Value = reverseOrderHebrew(c.Value)
   Next c
   Application.Calculation = xlAutomatic
End Sub


Function reverseOrderHebrew(ByVal str As String) As String

Dim str2 As String
str2 = ""
Dim engLettersCount As Integer
engLettersCount = 0
For i = Len(str) To 1 Step -1

   If engLetter(Mid(str, i, 1)) Then
        Do While ((i - engLettersCount) <> 0)
            If engLetter(Mid(str, i - engLettersCount, 1)) Then
                    engLettersCount = engLettersCount + 1
            Else
                Exit Do

            End If
        Loop
        'not english letter or end of string
        str2 = str2 + Mid(str, i - engLettersCount + 1, engLettersCount)
        i = i - engLettersCount + 1
        engLettersCount = 0
        getOut = 0
   Else
        str2 = str2 + Mid(str, i, 1)
   End If

Next i
reverseOrderHebrew = str2
End Function

Function engLetter(ByVal char As String) As Boolean

   If char < "א" Or char > "ת" Then
        engLetter = True
   Else
       engLetter = False
   End If
End Function

when i run this code on a pretty big selection the excel instance seems to get stuck and not respond. The code executes fine on smaller selections. The pc power is decent.

I know that my algorithm's efficiency is O(n^2) but it is still not supposed to stick the whole running process.

I suspect a memory leak but can't find it (garbage collector in vba?) Any help please?

Thanks!

Upvotes: 0

Views: 330

Answers (1)

Tim Williams
Tim Williams

Reputation: 166316

You can try this version of your algorithm - it's a little bit (~2x) faster in my testing (but I don't have any Hebrew text to test with, so I used the cutoffs shown...)

Function reverseOrderHebrew2(str As String) As String

Dim i As Long, c As String, sOut As String
Dim tmp As String

    i = Len(str)
    sOut = ""
    tmp = ""
    Do While i >= 1
        c = Mid(str, i, 1)
        If c < "a" Or c > "l" Then
            tmp = c & tmp
        Else
            If Len(tmp) > 0 Then
                sOut = sOut & tmp
                tmp = ""
            End If
            sOut = sOut & c
        End If
        i = i - 1
    Loop
    If Len(tmp) > 0 Then sOut = sOut & tmp

    reverseOrderHebrew2 = sOut
End Function

The other comments also have good suggestions - depending on how much text there is in your cells, the process of reading/writing the cell values may be more or less of a factor, and using a variant array may gain a lot of performance (as long as there are no formulas in your selection, which would get overwritten if you used that approach)

Upvotes: 1

Related Questions