Could someone review my first Excel VBA program?

[ElPresidente]

Hi everyone.This is my first attempt at coding in VBA for Excel. If
someone could give any tips about my code, I'd really appreciate it.

In a nutshell, this code reads a source spreadsheet of about 35,000
rows and pulls each sales manager out into a template file which
contains a pivot table. The template is then saved as a spreadsheet
with that manager's name. The code works, but I'm sure there's
something that can be improved. I thought it might be less memory
intensive if I could copy over an entire row into the template sheet.
But every time I tried writing code for that, I kept getting errors
preventing it from running. Thanks.

Dim varSourceRow As Integer 'Defines which row to read data from
source file
Dim varTemplateRow As Integer 'Defines which row to write data in
template file
Dim currentMgr As String 'Defines which manager is currently
being read
Dim pivotRange As String 'Defines data range for Pivot Table
in template
Dim varSourceLen As Integer 'Defines row count of the source
file



'Turn off screen updates
Application.ScreenUpdating = False

'Load the source data and template data into memory
Set sourceWB = Workbooks.Open("C:\Documents and Settings\Test
\Desktop\ReportTest\Source.xls", True, True)
Set templateWB = Workbooks.Open("C:\Documents and Settings\Test
\Desktop\ReportTest\Template.xls", True, True)

'Sets sales manager for first loop
currentMgr = sourceWB.Worksheets("Source Data").Cells(2, 3)
varTemplateRow = 2
varIteraion = 2
varSourceLen = sourceWB.Worksheets("Source
Data").UsedRange.Rows.Count


For r = 2 To varSourceLen

'If the row in the source document has the current sales manager
If sourceWB.Worksheets("Source Data").Cells(varSourceRow, 3).Value
= currentMgr Then
For clNumber = 1 To 16

'For each column copy over the cell data to the template file
templateWB.Worksheets("Sheet1").Cells(varTemplateRow,
clNumber).Value = sourceWB.Worksheets("Source
Data").Cells(varSourceRow, clNumber).Value
Next clNumber
varTemplateRow = varTemplateRow + 1
intCount = intCount + 1

'If the row in the source document does not have the current sales
manger
Else
'Define the pivot table range
pivotRange = "Sheet1!R1C1:R" & varTemplateRow - 1 & "C16"

'Refresh the pivot table with the new data

templateWB.Worksheets("Pivot").PivotTables("PivotTemp").ChangePivotCache
templateWB.PivotCaches.Create(SourceType:=xlDatabase,
SourceData:=pivotRange)

templateWB.Worksheets("Pivot").PivotTables("PivotTemp").RefreshTable

'Save the file using the sales manager name
templateWB.SaveAs "C:\Documents and Settings\Test\Desktop
\ReportTest\" & currentMgr & ".xls", FileFormat:=56
currentMgr = rawWb.Worksheets("Source
Data").Cells(varSourceRow, 3).Value

'Clear the contents of the template file for the next cycle
and reset the template row count
pivotRange = "A2" & varTemplateRow

templateWB.Worksheets("Sheet1").Range(pivotRange).ClearContents
varTemplateRow = 2
End If
Next rwNumber

'Once complete close the original files and remove variable values
sourceWB.Close False
templateWB.Close False
Set rawWb = Nothing
Set templateWB = Nothing
Application.ScreenUpdating = True

 

[ElPresidente]

Ugh it had the wrong one in my clipboard. Here is the correct one.

Dim varSourceRow As Integer 'Defines which row to read data from
source file
Dim varTemplateRow As Integer 'Defines which row to write data in
template file
Dim currentMgr As String 'Defines which manager is currently
being read
Dim pivotRange As String 'Defines data range for Pivot Table
in template
Dim varSourceLen As Integer 'Defines row count of the source
file



'Turn off screen updates
Application.ScreenUpdating = False

'Load the source data and template data into memory
Set sourceWb = Workbooks.Open("C:\Documents and Settings\Test
\Desktop\ReportTest\test.xls", True, True)
Set templateWB = Workbooks.Open("C:\Documents and Settings\Test
\Desktop\ReportTest\Template.xls", True, True)

'Sets sales manager for first loop
currentMgr = sourceWb.Worksheets("Source Data").Cells(2, 3)
varTemplateRow = 2
varSourceRow = 2
varSourceLen = sourceWb.Worksheets("Source
Data").UsedRange.Rows.Count


For r = 2 To varSourceLen

'If the row in the source document has the current sales manager
If sourceWb.Worksheets("Source Data").Cells(varSourceRow, 3).Value
= currentMgr Then
For clNumber = 1 To 16

'For each column copy over the cell data to the template file
templateWB.Worksheets("Sheet1").Cells(varTemplateRow,
clNumber).Value = sourceWb.Worksheets("Source
Data").Cells(varSourceRow, clNumber).Value
Next clNumber
varTemplateRow = varTemplateRow + 1
varSourceRow = varSourceRow + 1

'If the row in the source document does not have the current sales
manger
Else
'Define the pivot table range
pivotRange = "Sheet1!R1C1:R" & varTemplateRow - 1 & "C16"

'Refresh the pivot table with the new data

templateWB.Worksheets("Pivot").PivotTables("PivotTemp").ChangePivotCache
templateWB.PivotCaches.Create(SourceType:=xlDatabase,
SourceData:=pivotRange)

templateWB.Worksheets("Pivot").PivotTables("PivotTemp").RefreshTable

'Save the file using the sales manager name
templateWB.SaveAs "C:\Documents and Settings\Test\Desktop
\ReportTest\" & currentMgr & ".xls", FileFormat:=56
currentMgr = sourceWb.Worksheets("Source
Data").Cells(varSourceRow, 3).Value

'Clear the contents of the template file for the next cycle
and reset the template row count
pivotRange = "A2" & varTemplateRow

templateWB.Worksheets("Sheet1").Range(pivotRange).ClearContents
varTemplateRow = 2
End If
Next r

'Once complete close the original files and remove variable values
sourceWb.Close False
templateWB.Close False
Set rawWb = Nothing
Set templateWB = Nothing
Application.ScreenUpdating = True

 

[JLGWhiz]

My philosophy is that if it works, it is good code. You can learn about time
saving, space saving and other things that make for more efficient code as
you write more and try more things. If you have developed usable code, you
are successful.

 

[ElPresidente]

Well the problem is that I'm trying to bring it over to VB.net. It's
working there too but it's taking a significant amount of time more to
execute. If I run the above code from Excel, the whole thing executes
in about 10 seconds using 3,500 sample rows. When I bring it over to
VB in a simple form, each spreadsheet takes at least 10 seconds to
write. I suspect I'm either not copying the data in the best way or
something with how the workbooks are open.

 

[JLGWhiz]

I am not a speed demon, but then I do not do commercial programming. It
looks like you are copying a range one cell at a time. I would think that if
you copied
the entire range in one fell swoop, it would be a little faster. If the
range is dynamic then you would have to use variables for the start and end
of the range, but that is a minor tecnicality.