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

Importing a report through the enabler4excel add-in

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

Problem

This connects to Salesforce to import a report through the enabler4excel add-in. The report overwrites what is currently there on the sheet (2016 Incidents Dashboard). The report contains about 55,000 rows and 10 columns each. I also have 8 columns of formulas (nothing complex) off to the right. The next section autofills the Excel formulas from the last row on the right to match that of the report on the left.

This works, but the only problem is that it seems awfully slow: about 2 minutes. How can I do this so it doesn't take forever?

Sub ButtonClick1()

Dim addin As Office.COMAddIn
Dim automationObject As Object
Dim error As String
Dim error1 As String
Dim Result As Boolean
Dim ReportResult As String
Dim LastRowLeft As String
Dim LastRowRight As String
Dim sheetname As String

sheetname = ActiveSheet.Name
For Each addin In Application.COMAddIns
If addin.Description = "Enabler4Excel" Then
Set automationObject = addin.Object
End If
Next addin

Result = automationObject.LogIn("myemail.org", "mypassword",              "https://login.salesforce.com", error)
If Result = False Then
MsgBox error
End
End If

ReportResult = automationObject.RunReport("00Od0000004yulX", error1)

Dim wb As Excel.Workbook
Set wb = ThisWorkbook
Set ws = wb.Sheets("2016 Incidents Dashboard")

With ws.QueryTables.Add(Connection:="TEXT;" & ReportResult,           Destination:=ws.Range("A1"))
.TextFileParseType = xlDelimited
.TextFileCommaDelimiter = True
.RefreshStyle = xlOverwriteCells
.Refresh
End With

Result1 = automationObject.LogOut()

Worksheets("2016 Incidents Dashboard").Activate
LastRowLeft = ws.Range("A" & ws.Rows.Count).End(xlUp).Row
LastRowRight = ws.Range("M" & ws.Rows.Count).End(xlUp).Row
Range("M" & LastRowRight & ":U" & LastRowRight).Select
Selection.AutoFill Destination:=Range("M" & LastRowRight & ":U" & LastRowLeft)

ActiveWorkbook.RefreshAll
 Worksheets(sheetname).Activate
End Sub

Solution

Readability & Maintainability

  • Use indentation to make control-structures easier to understand and read.



  • Some variables are all lower-case, some are camelCase, some are PascalCase. OCD is unhappy :( Use a consistent naming convention. I'd recommend camelCase. Either way, choose one and stick to it



  • ButtonClick1 is a rather non-descriptive name



  • these "random" spaces before the third argument look strange. I assume they're not there in your own code, right?



Compartmentalization

You should be able to make your code into neat little compartments. Those are also known as "methods", "functions", "procedures" or a handful of other names. All these separate paragraphs that have been mentioned in a comment already can almost be methods. Consider:

Set automationObject = FindEnablerAddin()
' [...]
Private Function FindEnablerAddin() As Object
    For Each addin In Application.COMAddIns
        If addin.Description = "Enabler4Excel" Then
            Set FindEnablerAddin = addin.Object
            Exit Function
        End If
    Next addin
    'FindEnablerAddin should be Nothing
End Function


This can be applied in other places as well. Note that I have used Exit Function to break the loop over all COMAddIns early. This may or may not help with performance noticeably. It usually avoids work, so you should do it.
Performance & Nitpicks

  • You can usually significantly increase performance by turning of Events, Calculation and ScreenUpdating in Excel. Don't forget to turn them back on, even if your code falls into an error state.



  • Working on the Workbook directly is slow. Then again this looks like you don't have much of a choice here



  • Instead of If Result = False Then you can use If Not Result Then. This is cleaner and avoids the potential for confusion about assignment and boolean comparisons

Code Snippets

Set automationObject = FindEnablerAddin()
' [...]
Private Function FindEnablerAddin() As Object
    For Each addin In Application.COMAddIns
        If addin.Description = "Enabler4Excel" Then
            Set FindEnablerAddin = addin.Object
            Exit Function
        End If
    Next addin
    'FindEnablerAddin should be Nothing
End Function

Context

StackExchange Code Review Q#139739, answer score: 4

Revisions (0)

No revisions yet.