0

I have a VBA code which copies same data from Multiple sheet and then paste it in "Main" Sheet. It then auto fills the blank cells for values from above and then it delete all the rows Where H:H is blank. However being novice in VBA, i feel my code has too many loops, which makes it run slower. Moreover if have the "Main" Sheet have a table formatted, the code does not delete any row H is blank. However it works if "Main" is blank and not formatted.

Another thing I found out that after the code is executed, the excel sheet becomes less responsive. I cannot select cells quickly, change between sheets.

Please advise if anything can be improved to make it run more efficiently.

Private Sub CopyRangeFromMultiWorksheets1()



'Fill in the range that you want to copy
'Set CopyRng = sh.Range("A1:G1")

Dim sh As Worksheet
Dim DestSh As Worksheet
Dim rng As Range
Dim Last As Long
Dim CopyRng1 As Range
Dim CopyRng2 As Range
Dim CopyRng3 As Range
Dim CopyRng4 As Range
Dim CopyRng5 As Range
Dim CopyRng6 As Range
Dim CopyRng7 As Range
Dim cell As Range
Dim Row As Range
Dim LastrowDelete As Long



With Application
    .ScreenUpdating = False
    .EnableEvents = False
End With

'Delete the sheet "RDBMergeSheet" if it exist
'Application.DisplayAlerts = False
On Error Resume Next
'ActiveWorkbook.Worksheets("RDBMergeSheet").Delete
On Error GoTo 0
'Application.DisplayAlerts = True

'Add a worksheet with the name "RDBMergeSheet"
 Set DestSh = Sheets("Main")
'Set DestSh = ActiveWorkbook.Worksheets.Add
' DestSh.Name = "RDBMergeSheet"

'loop through all worksheets and copy the data to the DestSh
 For Each sh In ActiveWorkbook.Worksheets
    If sh.Name <> DestSh.Name And sh.Name <> "PAYPERIOD" And sh.Name <> 
  "TECHTeamList" Then

        'Find the last row with data on the DestSh
        Last = LastRow(DestSh)

        'Fill in the range that you want to copy
        Set CopyRng1 = sh.Range("B3")
        Set CopyRng2 = sh.Range("C3")
        Set CopyRng3 = sh.Range("D3")
        Set CopyRng4 = sh.Range("G3")
        Set CopyRng5 = sh.Range("C5")
        Set CopyRng6 = sh.Range("A8:j25")
        Set CopyRng7 = sh.Range("A28:j45")

        'Test if there enough rows in the DestSh to copy all the data
        If Last + CopyRng1.Rows.Count > DestSh.Rows.Count Then
            MsgBox "There are not enough rows in the Destsh"
            GoTo ExitTheSub
        End If

        'This example copies values/formats, if you only want to copy the
        'values or want to copy everything look at the example below this 
  macro
        CopyRng1.Copy
        With DestSh.Cells(Last + 1, "A")
            .PasteSpecial xlPasteValues
            Application.CutCopyMode = False
        End With
        CopyRng2.Copy
        With DestSh.Cells(Last + 1, "B")
            .PasteSpecial xlPasteValues
            Application.CutCopyMode = False
        End With
        CopyRng3.Copy
        With DestSh.Cells(Last + 1, "C")
            .PasteSpecial xlPasteValues
            Application.CutCopyMode = False
        End With

         CopyRng4.Copy
        With DestSh.Cells(Last + 1, "D")
            .PasteSpecial xlPasteValues
            Application.CutCopyMode = False

        End With
         CopyRng5.Copy
        With DestSh.Cells(Last + 1, "E")
            .PasteSpecial xlPasteValues
            Application.CutCopyMode = False
        End With


        CopyRng6.Copy
        With DestSh.Cells(Last + 1, "F")
            .PasteSpecial Paste:=xlPasteValuesAndNumberFormats
            Application.CutCopyMode = False
        End With


        'Refresh the Lastrow used so that the values start from 
        'underneath copyrng6

        Last = LastRow(DestSh)
         CopyRng7.Copy
        With DestSh.Cells(Last + 1, "F")
            .PasteSpecial Paste:=xlPasteValuesAndNumberFormats
            Application.CutCopyMode = False
        End With


    End If
Next

ExitTheSub:

Application.Goto DestSh.Cells(1)

'AutoFit the column width in the DestSh sheet
 DestSh.Columns.AutoFit

 'Autofill the rang A2:E for values from above looking at  the last row of F
 With Range("A2:E" & Range("F" & Rows.Count).End(xlUp).Row)
 .SpecialCells(xlBlanks).FormulaR1C1 = "=R[-1]C"
.Value = .Value
End With


 'Delete Entire rows where H is Blank
Application.ScreenUpdating = False
Columns("H:H").SpecialCells(xlCellTypeBlanks).EntireRow.Delete
Application.ScreenUpdating = True

With Application
    .ScreenUpdating = True
    .EnableEvents = True
 End With
End Sub

Function LastRow(sh As Worksheet)
On Error Resume Next
LastRow = sh.Cells.Find(What:="*", _
                        After:=sh.Range("A1"), _
                        Lookat:=xlPart, _
                        LookIn:=xlFormulas, _
                        SearchOrder:=xlByRows, _
                        SearchDirection:=xlPrevious, _
                        MatchCase:=False).Row
On Error GoTo 0
End Function

Any Help would be appreciated.

Martijn Pieters
  • 1,048,767
  • 296
  • 4,058
  • 3,343
Nav kaur
  • 17
  • 4
  • 1
    [Avoiding copy paste](https://stackoverflow.com/questions/51405731/avoiding-select-with-copy-and-paste-vba) would help – cybernetic.nomad Aug 24 '18 at 15:36
  • I tried other things but this was the only code i got it to working. So you can help or elaborate, it will be highly appreciated. Thanks – Nav kaur Aug 24 '18 at 15:41
  • Your code has exactly one loop in it. `Application.CutCopyMode = False` does not make much sense to me. Does it actually do something for your script? Same goes for the with statement, what you are actually doing is just `DestSh.Cells(Last + 1, "A").PasteSpecial xlPasteValues`. Also, if it is just Ranges of a few, or even a single value (as most of your ranges), reading and writing `.value` will definitely be faster than copy-pasting. – Franz Aug 24 '18 at 15:41
  • Really appreciate for the feedback, I will remove Application.CutCopyMode = False and then use .Values. Another question, my sheet is not responding even after the code is executed. I cannot select any cell. DO you think it has anything to do with screen updating. – Nav kaur Aug 24 '18 at 15:45
  • Thanks Franz. Removing Application.CutCopyMode = False has definitely helped. Sorry but I have no idea how should I change it to adapt to .value. – Nav kaur Aug 24 '18 at 15:52
  • For example, `DestSh.Cells(Last + 1, "A").Value = sh.Range("B3").Value` – Marcucciboy2 Aug 24 '18 at 16:05
  • This looks like it would be better suited to [Code Review](https://codereview.stackexchange.com/). StackOverflow is for broken code, Code Review is for "how do I improve this working code". – Darren Bartrup-Cook Aug 24 '18 at 16:06
  • Thanks Darren, I did not knew that we can have our code reviewed. – Nav kaur Aug 24 '18 at 16:10

0 Answers0