1

I have a Excel workbook, almost like a database, where I update Historical data each week. Using a separate sub, I pull in an Export as a worksheet to the book. I find the unique dates that are in the export. I then look at Historical data, and if the Historical date matches one of the Export dates, I delete the row in Historical. Eventually I copy and paste the Export in to the Historical data tab.

The code below works how I'd like it to, but I have some questions after the block of code:

Sub AddNewData()

'This will take what's in Export and put it in to Historical

Dim Historical As Worksheet
Dim Export As Worksheet
Dim exportdates As Range

Set Historical = ThisWorkbook.Worksheets("Historical")
Set Export = ThisWorkbook.Worksheets("Export")

'Pulling unique values of dates from this range and pasting to M1:
Export.Range("B2:B" & Export.Cells(Export.Rows.Count, 1).End(xlUp).Row).AdvancedFilter _
    Action:=xlFilterCopy, CopyToRange:=Export.Range("M1"), Unique:=True

'Originally I was thinking I could make this a list of some sort vlookup or match?
'As of now, though, it goes unused...:
Set exportdates = Export.Range("M1:M" & Export.Cells(Export.Rows.Count, 13).End(xlUp).Row)

For r = Historical.Cells(Rows.Count, 1).End(xlUp).Row To 1 Step -1
    If Historical.Cells(r, 2).Value = exportdates(1, 1).Value Or _
        Historical.Cells(r, 2).Value = exportdates(2, 1).Value Or _
        Historical.Cells(r, 2).Value = exportdates(3, 1).Value _
        Then Historical.Rows(r).Delete
Next

'Copying and pasting Export data to Historical tab
Export.Range("A2:J" & Export.Cells(Export.Rows.Count, 1).End(xlUp).Row).Copy
Historical.Range("A" & Historical.Cells(Historical.Rows.Count, 1).End(xlUp).Row + 1).PasteSpecial xlPasteValues

Application.CutCopyMode = False

End Sub

1) Can that IF statement be condensed somehow using the exportdates range?

2) This works just fine for a few hundred rows of data when my dates are simply the first of each month, but I also have an export that has each day as a date that I'll have to match with a different tab with daily information. That one has THOUSANDS of rows. I don't believe this macro will be much more efficient than simply sorting by date and eliminating myself? Can I change the IF statement to be more inclusive, like question 1?

Thank you!

braX
  • 11,506
  • 5
  • 20
  • 33

1 Answers1

0

Whenever you have to delete many rows in Excel with VBA, the best practice is to assign these rows to a range and to delete the range at the end.

Thus, your code should be refactored in this part:

For r = Historical.Cells(Rows.Count, 1).End(xlUp).Row To 1 Step -1
    If Historical.Cells(r, 2).Value = exportdates(1, 1).Value Or _
        Historical.Cells(r, 2).Value = exportdates(2, 1).Value Or _
        Historical.Cells(r, 2).Value = exportdates(3, 1).Value _
        Then Historical.Rows(r).Delete
Next

This is a simple sample that you can use for the refactoring (just make sure to write a few times 1 in Range("A1:A20") to see how it works:

Public Sub TestMe()

    Dim deleteRange As Range
    Dim cnt         As Long

    For cnt = 20 To 1 Step -1
        If Cells(cnt, 1) = 1 Then
            If Not deleteRange Is Nothing Then
                Set deleteRange = Union(deleteRange, Cells(cnt, 1))
            Else
                Set deleteRange = Cells(cnt, 1)
            End If
        End If
    Next cnt

    deleteRange.EntireRow.Select
    Stop
    deleteRange.EntireRow.Delete

End Sub

Once you run the code it stops at the Stop sign. You see that the rows to be deleted are selected. Once you continue with F5 they would be deleted. Consider removing the Stop and .Select line in your code.

Some general ideas how to speed up code: https://stackoverflow.com/a/49514930/5448626

Vityata
  • 42,633
  • 8
  • 55
  • 100
  • This works! Instead of using: If Cells(cnt, 1) = 1 or Cells(cnt, 1) = 2 or Cells(cnt, 1) = 4 Then Is there any way for me to say something like IF Cells(cnt, 1) = In a simple line of code? – yourleftleg Mar 28 '18 at 15:11
  • @yourleftleg - congrats! :) If you have 3 conditions you need 3 conditions. – Vityata Mar 28 '18 at 16:31
  • 1
    I actually ended up figuring out that I could essentially do iserror(Application.Match(CLng(.Cells(cnt, 2)), famedates, 0)) as the one line of code to check my exportdates range. That worked as well! – yourleftleg Mar 28 '18 at 16:52
  • @yourleftleg - this is probably better, but thus you have to take care of a new variable - "famedates". – Vityata Mar 28 '18 at 17:10