patternMinor
Importing a report through the enabler4excel add-in
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?
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 SubSolution
Readability & Maintainability
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:
This can be applied in other places as well. Note that I have used
Performance & Nitpicks
- 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
ButtonClick1is 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 FunctionThis 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 Thenyou can useIf 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 FunctionContext
StackExchange Code Review Q#139739, answer score: 4
Revisions (0)
No revisions yet.