HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Excel sheet code is taking too much time

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
muchexcelsheettootimecodetaking

Problem

I have an Excel sheet with lot of rows with data. I am separating it into 2 sheets with some condition satisfies.

This is taking too much time. How can I improve the speed? Please suggest any changes to my code.

```
Dim bmver As Object(,) = New Object(i20, 36) {}
Dim bmhor As Object(,) = New Object(i20, 36) {}

Dim ik As Integer = 0
Dim ij As Integer = 0
With oWs

For i As Integer = 2 To i20 - 1
If (.range("E" & i).value = .range("L" & i).value) Then

For j As Integer = 0 To 36
bmhor(ij, j) = .cells(i, j + 1).value
Next
ij = ij + 1
End If

Next

For k As Integer = 2 To i20 - 1
If (.range("C" & k).value = .range("J" & k).value) Then

For j1 As Integer = 0 To 36
bmver(ik, j1) = .cells(k, j1 + 1).value
Next
ik = ik + 1
End If

Next

End With
oWs = oBook.Worksheets("beamhor")
oWs.activate()
Dim r6 As Range = oWs.Range("A2").Resize(bmhor.GetLength(0), 37)
r6.Value2 = bmhor

With oWs

.Range("A2:AE" & i20).Sort(Key1:=.range("E2"), Order1:=Excel.XlSortOrder.xlAscending, _
Key2:=.range("L2"), Order2:=Excel.XlSortOrder.xlAscending, _
Key2:=.range("C2"), Order2:=Excel.XlSortOrder.xlAscending, _
Key2:=.range("J2"), Order2:=Excel.XlSortOrder.xlAscending, _
Orientation:=XlSortOrientation.xlSortColumns, _
Header:=Excel.XlYesNoGuess.xlNo, _
SortMethod:=XlSortMethod.xlPinYin)

End With
oWs = oBook.Worksheets("beamver")
oWs.activate()
Dim r7 As Range = oWs.Range("A2").Resize(bmver.GetLength(0), 37)
r7.Value2 = bmver
With oWs

.Range("A2:AE" & i20).Sort(Key1:=.range("E2"), Order1:=Excel.XlSortOrder.xlAscending, _
Key2:=.range("L2"), Order2:=Excel.XlSortOrder.xlAscending, _
Key2:=.range("C2"), Order2:=Excel.XlSortOrder.xlAscending, _
Key2:=.range("J2"), Order2:=Excel.XlSortOrder.xlAscending, _

Solution

Excel interoperability is inherently slow - you have the .NET runtime talking to a COM object model through COM interop, there's a performance penalty just for doing that.

Hence, you should strive to reduce the number of times you're accessing the worksheet cells, be it to read or to write.

Before going into the algorithm, I'd like to point out the naming issues. Bad names make the code harder to read and longer to parse. Typing code is 20% of the job - the other 80% is spent reading code, so when the first 20% makes a good job at being readable, the 80% can be more productive.

Avoid single-letter identifiers (for-loop variable is ok though) and disemvoweling. Use comments - don't write code that assumes the reader is also looking at the worksheet in question: name things so that we know what columns C, E, J and L are, or comment to that effect.

Now. We have two loops, both iterating from 2 To i20 - 1. Round one would be to merge them:

For i As Integer = 2 To i20 - 1

    If (.range("E" & i).value = .range("L" & i).value) Then

        For j As Integer = 0 To 36
            bmhor(ij, j) = .cells(i, j + 1).value
        Next

        ij = ij + 1

    End If

    If (.range("C" & i).value = .range("J" & i).value) Then

        For j As Integer = 0 To 36 'I'd believe identifier j can be reused here
            bmver(ik, j) = .cells(i, j + 1).value
        Next

        ik = ik + 1

    End If

Next


Just doing that has reduced the number of iterations by half. Now these two blocks look awfully similar. Let's see if we can extract a method here:

Sub NameMe(Value1 As String, Value2 As String, Row As Long, TheWorksheet As Worksheet, TheArray As Object(,))

    Dim Counter As Integer
    If (Value1 = Value2) Then

        For i As Integer = 0 To 36
            TheArray(Counter, i) = TheWorksheet.Cells(Row, i + 1).Value
        Next

        Counter += 1

    End If

End Sub


Then the loop can look like this:

Dim ValueForColumnE As String
Dim ValueForColumnL As String
Dim ValueForColumnC As String
Dim ValueForColumnJ As String

For i As Integer = 2 To i20 - 1

    ValueForColumnE = .range("E" & i).Value.ToString()
    ValueForColumnL = .range("L" & i).Value.ToString()
    ValueForColumnC = .range("C" & i).Value.ToString()
    ValueForColumnJ = .range("J" & i).Value.ToString()

    NameMe ValueForColumnE, ValueForColumnL, i, ows, bmhor
    NameMe ValueForColumnC, ValueForColumnJ, i, ows, bmver

Next

Code Snippets

For i As Integer = 2 To i20 - 1

    If (.range("E" & i).value = .range("L" & i).value) Then

        For j As Integer = 0 To 36
            bmhor(ij, j) = .cells(i, j + 1).value
        Next

        ij = ij + 1

    End If

    If (.range("C" & i).value = .range("J" & i).value) Then

        For j As Integer = 0 To 36 'I'd believe identifier j can be reused here
            bmver(ik, j) = .cells(i, j + 1).value
        Next

        ik = ik + 1

    End If

Next
Sub NameMe(Value1 As String, Value2 As String, Row As Long, TheWorksheet As Worksheet, TheArray As Object(,))

    Dim Counter As Integer
    If (Value1 = Value2) Then

        For i As Integer = 0 To 36
            TheArray(Counter, i) = TheWorksheet.Cells(Row, i + 1).Value
        Next

        Counter += 1

    End If

End Sub
Dim ValueForColumnE As String
Dim ValueForColumnL As String
Dim ValueForColumnC As String
Dim ValueForColumnJ As String

For i As Integer = 2 To i20 - 1

    ValueForColumnE = .range("E" & i).Value.ToString()
    ValueForColumnL = .range("L" & i).Value.ToString()
    ValueForColumnC = .range("C" & i).Value.ToString()
    ValueForColumnJ = .range("J" & i).Value.ToString()

    NameMe ValueForColumnE, ValueForColumnL, i, ows, bmhor
    NameMe ValueForColumnC, ValueForColumnJ, i, ows, bmver

Next

Context

StackExchange Code Review Q#48291, answer score: 3

Revisions (0)

No revisions yet.