AlexO_Leary
New member
- Local time
- Today, 11:15
- Joined
- Oct 24, 2019
- Messages
- 4
I've got two Worksheets in Excel. I've written the following code to copy some data from Worksheet 1 to Worksheet 2, based on some values that the user inserts in Worksheet 2.
The macro works fine, and does what I need it to do, but after writing it down I've come to realize two things:
It takes quite some time for a small set of records(260 or so), as it goes one row at a time.
I read that using .select is not good practice, and I modified the code so that I would not use it, but I'm left wondering if I could improve the code to work faster if I did use it.
So, my main questions are:
How can I improve the speed of the code, so that it will be able to read copy rows faster.
Would it be better in this case to use .select in my case, so that it would work faster.
My code is the following:
Any help or opinion is very welcome, as I am quite new to the whole VBA world.
Thanks!!
The macro works fine, and does what I need it to do, but after writing it down I've come to realize two things:
It takes quite some time for a small set of records(260 or so), as it goes one row at a time.
I read that using .select is not good practice, and I modified the code so that I would not use it, but I'm left wondering if I could improve the code to work faster if I did use it.
So, my main questions are:
How can I improve the speed of the code, so that it will be able to read copy rows faster.
Would it be better in this case to use .select in my case, so that it would work faster.
My code is the following:
Code:
Private Sub FillUp()
Dim DateVal, EquivalentDate As Date
Dim CrncyVal
Dim CountrVal
Dim DataRng As Range
Dim endrow As Long, startrow As Long
Dim ws1 As Worksheet
Dim ws2 As Worksheet
'Selecting the worksheets
Set ws1 = Worksheets("Sheet1")
Set ws2 = Worksheets("Sheet2")
''''declaring date, country and currency variables''''
DateVal = ws2.Range("E3").Value
CountryVal = UCase(ws2.Range("H3").Value)
CurrencyVal = UCase(ws2.Range("H4").Value)
EquivalentDateVal = DateAdd("yyyy", -1, DateVal)
'declaring other useful variables
startrow = 3
pasterow = 6
endrow = ws1.Cells(Rows.Count, 1).End(xlUp).Row
'delete the range we will be working with
ws2.Range("A6:F265").Clear
'start the ifs, to see what info the user wants to get
If ws2.Range("E3").Value = "" Then
'If the country cell is empty, we do nothing. We need at least this info
MsgBox prompt:="You didn't choose a month!!", Title:="Error: Please Choose a Month"
Exit Sub
ElseIf ws2.Range("H3").Value = "" Then
For i = 3 To endrow
If ws1.Cells(i, 3).Value <> "TOT" Then
With ws1
Set Rng = .Range(.Cells(i, 1), .Cells(i, 5))
End With
Rng.Copy
ws2.Cells(pasterow, 1).PasteSpecial
ws2.Cells(pasterow, 6) = DateVal
pasterow = pasterow + 1
End If
Next i
Exit Sub
ElseIf ws2.Range("H4").Value = "" Then
For i = 3 To endrow
If ws1.Cells(i, 3).Value <> "TOT" Then
If ws1.Cells(i, 1).Value = CountryVal Then
With ws1
Set Rng = .Range(.Cells(i, 1), .Cells(i, 5))
End With
Rng.Copy
ws2.Cells(pasterow, 1).PasteSpecial
ws2.Cells(pasterow, 6) = DateVal
pasterow = pasterow + 1
End If
End If
Next i
Exit Sub
Else
For i = 3 To endrow
If ws1.Cells(i, 3).Value <> "TOT" Then
If ws1.Cells(i, 1).Value = CountryVal Then
If ws1.Cells(i, 2).Value = CurrencyVal Then
With ws1
Set Rng = .Range(.Cells(i, 1), .Cells(i, 5))
End With
Rng.Copy
ws2.Cells(pasterow, 1).PasteSpecial
ws2.Cells(pasterow, 6) = DateVal
pasterow = pasterow + 1
End If
End If
End If
Next i
Exit Sub
End If
End SubAny help or opinion is very welcome, as I am quite new to the whole VBA world.
Thanks!!
