patternMinor
Copying a range from one file to another
Viewed 0 times
filerangeonecopyinganotherfrom
Problem
Since it has too many loop, loop inside loop, this works very slowly. How can we increase the speed of this program? I am trying to copy a range from one file to another file of same name in different folder. I have folders named
"A","B","C"..."G" inside c:\Charts\1\ and c:\Charts\0\. Inside each folder "A","B","C"..."G" there are files named 1,2,3,4,..10.dim vArr1 as array
Dim vArr as array
vArr1 = Array("A", "B", "C", "D", "E", "F", "G")
vArr = Array("A", "B", "C", "D", "E", "F", "G")
Dim fileName1, Pathname1 As String
Pathname1 = "c:\Charts\1\"
Pathname="c:\charts\0\"
For Each vFile1 In vArr1
fileName1 = Dir(Pathname1 & vFile1 & "\" & "*.xlsx")
Do While fileName1 <> ""
For Each vFile In vArr
filename = Dir(Pathname & vFile & "\" & "*.xlsx")
Do While filename <> ""
If filename = fileName1 Then
Set WB1 = Workbooks.Open(Pathname1 & vFile & "\" & fileName1)
WB1.Application.ScreenUpdating = False
WB1.ActiveSheet.Range("M1:M30").Copy
WB1.Close (False)
Set WBD1 = Workbooks.Open(Pathname & vFile & "\" & filename)
WBD1.ActiveSheet.Range("C1").Select
WBD1.ActiveSheet.Paste
WBD1.ActiveSheet.Cells(1, 3).Value = "HSI Q4 2014-15"
WBD1.Close (True)
filename = Dir()
Else
End If
fileName1 = filename
Loop
Next
Loop
NextSolution
I doubt this is how you've actually declared arrays:
That would be
But nevermind that: you need to use more meaningful names. If they're folder names, consider calling them something like
This is why multiple declarations in the same instruction should be avoided:
Do you think
Indentation is off here, and only looks the way it does because
Should be:
Note that your usage of parenthesis in method calls is going to cause a bug one day or another, when you pass an argument like that into a
How so? Well, the "normal" VBA syntax for passing arguments is like this:
By doing this:
You are forcing the argument to be passed
...or even for a
The main reason why your code is slow is because you're opening and closing workbooks - there's not really anything to do to speed that up unfortunately.
Except perhaps...
Take that part out of the loop, and turn off screen updating all the while.
Now, whenever you turn off screen updating, you need to turn it back on when you're done, and you need to handle runtime errors and make sure you're turning it back on even when an error breaks you out of the code. You're accessing the file system here, so you have to handle runtime errors.
See this post for how to properly handle errors in VBA.
dim vArr1 as array
Dim vArr as arrayThat would be
Dim vArr1() As String
Dim vArr() As StringBut nevermind that: you need to use more meaningful names. If they're folder names, consider calling them something like
folders or folderNames - and avoid arbitrary Hungarian-like prefixes such as v.This is why multiple declarations in the same instruction should be avoided:
Dim fileName1, Pathname1 As StringDo you think
fileName1 is a String? It's not. It's implicitly declared as Variant, because As String only applies to Pathname1!Indentation is off here, and only looks the way it does because
WB1 has 3 characters (and it's another bad name):Set WB1 = Workbooks.Open(Pathname1 & vFile & "\" & fileName1)
WB1.Application.ScreenUpdating = False
WB1.ActiveSheet.Range("M1:M30").Copy
WB1.Close (False)Should be:
Set WB1 = Workbooks.Open(Pathname1 & vFile & "\" & fileName1)
WB1.Application.ScreenUpdating = False
WB1.ActiveSheet.Range("M1:M30").Copy
WB1.Close (False)Note that your usage of parenthesis in method calls is going to cause a bug one day or another, when you pass an argument like that into a
ByRef parameter.How so? Well, the "normal" VBA syntax for passing arguments is like this:
WB1.Close FalseBy doing this:
WB1.Close (False)You are forcing the argument to be passed
ByVal, overriding the procedure's signature that might have said ByRef. Of course it doesn't really matter here for a Boolean literal..Workbooks.Open(Pathname1 & vFile & "\" & fileName1)...or even for a
String literal. But one day you're going to want to pass an argument into a ByRef parameter, and learn the hard way that parentheses should be dropped when doing procedure calls. They're only needed for function calls.The main reason why your code is slow is because you're opening and closing workbooks - there's not really anything to do to speed that up unfortunately.
Except perhaps...
WB1.Application.ScreenUpdating = FalseTake that part out of the loop, and turn off screen updating all the while.
Application.ScreenUpdating = FalseNow, whenever you turn off screen updating, you need to turn it back on when you're done, and you need to handle runtime errors and make sure you're turning it back on even when an error breaks you out of the code. You're accessing the file system here, so you have to handle runtime errors.
See this post for how to properly handle errors in VBA.
Code Snippets
dim vArr1 as array
Dim vArr as arrayDim vArr1() As String
Dim vArr() As StringDim fileName1, Pathname1 As StringSet WB1 = Workbooks.Open(Pathname1 & vFile & "\" & fileName1)
WB1.Application.ScreenUpdating = False
WB1.ActiveSheet.Range("M1:M30").Copy
WB1.Close (False)Set WB1 = Workbooks.Open(Pathname1 & vFile & "\" & fileName1)
WB1.Application.ScreenUpdating = False
WB1.ActiveSheet.Range("M1:M30").Copy
WB1.Close (False)Context
StackExchange Code Review Q#94550, answer score: 8
Revisions (0)
No revisions yet.