3
\$\begingroup\$

I have a program that read thousands of lines and return the value that I need. The problem is that request take a lot of time probably like 1min just to search and paste the value, even when I'm saving the code it takes a lot of time...

I think it is around this line that the code is slow :

If Sheets("MT950").Cells(line, 1) Like "-}{5:*" Then

Here is my code :

Function mysolde62(mycurrency As String, swift As String) As Double

Dim SearchString As String
Dim LastLine As Long, line As Long, k As Long
Dim mybegin As Long, myend As Long, test As Long, count As Long
Dim sign As String
Dim myvalue As Double

LastLine = Sheets("MT950").Range("A1").End(xlDown).Row

count = 0
myend = 1

For line = 1 To LastLine

If Sheets("MT950").Cells(line, 1) Like "-}{5:*" Then

SearchString = Sheets("MT950").Range("A" & line).Value

mypos = InStr(1, SearchString, swift, 1)
If mypos <> 0 Then
    count = count + 1
End If

End If
Next line

For k = 1 To count

For line = myend To LastLine
If Sheets("MT950").Cells(line, 1) Like "-}{5:*" Then
    SearchString = Sheets("MT950").Range("A" & line).Value

    mypos = InStr(1, SearchString, swift, 1)
    If mypos <> 0 Then

        mybegin = line
        For linebis = mybegin To LastLine
            If Sheets("MT950").Cells(linebis, 1) Like ":62F:*" Then
                SearchString = Sheets("MT950").Range("A" & linebis).Value

                mypos = InStr(1, SearchString, mycurrency, 1)
                If mypos <> 0 Then
                    myend = linebis
                    test = 1
                End If
                Exit For
            End If
        Next linebis

        If test = 1 Then Exit For
    End If
End If
Next line
If test = 1 Then Exit For
Next k


sign = Mid(Sheets("MT950").Cells(myend, 1).Value, 5, 1)
myvalue = Mid(Sheets("MT950").Cells(myend, 1).Value, 15)
If sign = "D" Then
mysolde62 = -myvalue
Else
mysolde62 = myvalue
End If


End Function
\$\endgroup\$
1
\$\begingroup\$

Important notes:

  • Proper indenting helps make code easier to read and maintain.
  • Always use Option Explicit

The first area of performance improvement is to use arrays, not keep referring to Excel objects. As an example - your first loop is:

LastLine = Sheets("MT950").Range("A1").End(xlDown).Row

count = 0
myend = 1

For line = 1 To LastLine

If Sheets("MT950").Cells(line, 1) Like "-}{5:*" Then

SearchString = Sheets("MT950").Range("A" & line).Value

mypos = InStr(1, SearchString, swift, 1)
If mypos <> 0 Then
    count = count + 1
End If

End If
Next line

Where as, it could be:

LastLine = Sheets("MT950").Range("A1").End(xlDown).Row
Dim tempArray as Variant '<-- this is where we hold the values.
tempArray = Sheets("MT950").range("A1:A" & CStr(LastLine)).Value
count = 0
myend = 1

For line = LBound (tempArray, 1) to UBound(tempArray,1) ' Cycle through array
    If tempArray(line,1) Like "-}{5:*" Then
        mypos = InStr(1, tempArray(line,1), swift, 1)
        If mypos <> 0 Then
            count = count + 1
        End If
    End If
Next line

Your k loop does not appear to achieve anything, except repeat something without any variation [until deeper inspection, myend varies]. The performance of inner portion of that loop can also be improved by the use of arrays.

Looking at the code, this uses exactly the same array we used last time - so no need to even re-assign it!

For k = 1 To count

For line = myend To LastLine
If Sheets("MT950").Cells(line, 1) Like "-}{5:*" Then
    SearchString = Sheets("MT950").Range("A" & line).Value

    mypos = InStr(1, SearchString, swift, 1)
    If mypos <> 0 Then

        mybegin = line
        For linebis = mybegin To LastLine
            If Sheets("MT950").Cells(linebis, 1) Like ":62F:*" Then
                SearchString = Sheets("MT950").Range("A" & linebis).Value

                mypos = InStr(1, SearchString, mycurrency, 1)
                If mypos <> 0 Then
                    myend = linebis
                    test = 1
                End If
                Exit For
            End If
        Next linebis

        If test = 1 Then Exit For
    End If
End If
Next line
If test = 1 Then Exit For
Next k

Can be like:

For k = 1 To count
    For line = myend To LastLine
      If tempArray(line, 1) Like "-}{5:*" Then
          mypos = InStr(1, tempArray(line, 1), swift, 1)
          If mypos <> 0 Then
              mybegin = line
              For linebis = mybegin To LastLine
                  If tempArray(linebis , 1) Like ":62F:*" Then
                      mypos = InStr(1, tempArray(linebis , 1), mycurrency, 1)
                      If mypos <> 0 Then
                          myend = linebis
                          test = 1
                      End If
                   Exit For
                   End If
               Next linebis
           If test = 1 Then Exit For
       End If
    End If
    Next line
    If test = 1 Then Exit For
Next k

Notice that with these changes, we only touch the Excel model only once - to extract the array of values and this will greatly improve performance. With some re-thinking, you can have the required Range as a parameter - thus allowing this to be re-used or flexible (perhaps even as a UDF, depending on where you are getting your "swift" and "currency" values from!).

With these simple changes in place, you can look at your code logic and determine if other optimisations can be done.

\$\endgroup\$
  • \$\begingroup\$ Sorry for the late reply ! I have tried your changes this morning, it works way faster now, thank you very much ! \$\endgroup\$ – guenoon Jul 1 at 7:54
1
\$\begingroup\$

Off the bat it appears that you are looping through the entire sheet "MT950" twice. Doing this once will improve the speed. Also, the line you said it might be slow around appears twice in your code.

In your first loop you are running two comparisons, one using the If, Like, Then and then another right after using InStr(1, SearchString, swift, 1). These can be combined into the following

For Line = 1 To LastLine
    SearchString = Sheets("MT950").Range("A" & Line).Value
    If InStr(1, SearchString, swift, 1) <> 0 and SearchString Like "-}{5:*" Then Count = Count + 1
Next Line

The second loop appears to be the same as the first only with more stuff happening once you get down inside of it. I'd recommend merging these two loops, there doesn't seem to be a need to loop through all the rows twice.

\$\endgroup\$
  • \$\begingroup\$ Won't this capture lines that are NOT Like "-}{5:*"? \$\endgroup\$ – Shawn V. Wilson Jul 9 at 2:15
  • 1
    \$\begingroup\$ You are correct, the like operator compares a string to a pattern. OP compares the value in the cell that becomes SearchString against that pattern, and then checks to see if swift is contained in this cell. The above makes one check, if SearchString contains swift, and foregoes the pattern check since we are looking at the value of SearchString anyway. Admittedly this is not a great answer because it does not exclude those that don't follow the prescribed pattern, which OP likely needs since it was included originally. Edited to include pattern in If statement. Good catch. \$\endgroup\$ – DoomedJupiter Jul 9 at 19:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.