Question about optimizing simple code

hitbid · Nov 3, 2018

Hello,

This is just a simple VBA code I made to import data from three tabs, and bring it all into one tab where work can then be done on the combined data.

This will normally run on data that runs about 18-30k rows per tab. I like the code, and feel that it is sufficient, but if anyone has any advice on how to make it better I am all ears. Definitely not an urgent post that I need answered, but if you have any suggestions to make it faster, or smarter, thanks.

Code:

Dim B_Row1 As Long
Dim B_Row2 As Long

Private Sub BR1()
B_Row1 = Cells(Rows.Count, 1).End(xlUp).Row
End Sub

Private Sub BR2()
B_Row2 = Cells(Rows.Count, 1).End(xlUp).Row
End Sub

Sub Master_Finder()

Application.ScreenUpdating = False
Sheets("1").Activate
BR1
Range(Cells(1, 1), Cells(B_Row1, 2)).Copy Sheets("Test").Cells(2, 1)
Sheets("Test").Activate
BR2
Sheets("2").Activate
BR1
Range(Cells(1, 1), Cells(B_Row1, 2)).Copy Sheets("Test").Cells(B_Row2 + 1, 1)
Sheets("Test").Activate
BR2
Sheets("3").Activate
BR1
Range(Cells(1, 1), Cells(B_Row1, 2)).Copy Sheets("Test").Cells(B_Row2 + 1, 1)

Sheets("Test").Activate
ActiveWindow.ScrollRow = 1
Application.ScreenUpdating = True

End Sub

Fluff · Nov 3, 2018

I'd tend to do it like

Code:

Function B_Row1(Sht As String)
B_Row1 = Sheets(Sht).Cells(Rows.Count, 1).End(xlUp).Row
End Function

Function B_Row2(Sht As String)
B_Row2 = Sheets(Sht).Cells(Rows.Count, 1).End(xlUp).Offset(1).Row
End Function

Sub Master_Finder()

Application.ScreenUpdating = False
With Sheets("sheet1")
   .Range(.Cells(1, 1), .Cells(B_Row1(.Name), 2)).Copy Sheets("Test").Cells(2, 1)
End With
With Sheets("sheet2")
   .Range(.Cells(1, 1), .Cells(B_Row1(.Name), 2)).Copy Sheets("Test").Cells(B_Row2("Test"), 1)
End With
With Sheets("sheet3")
   .Range(.Cells(1, 1), .Cells(B_Row1(.Name), 2)).Copy Sheets("Test").Cells(B_Row2("Test"), 1)
End With
Sheets("Test").Activate
ActiveWindow.ScrollRow = 1
Application.ScreenUpdating = True

End Sub

hitbid · Nov 3, 2018

Thanks for the reply. Creating the function will help optimize in this case, or is that how you like to format things like this? I assume running the function is faster than calling on the separate Subs I have?

Fluff · Nov 3, 2018

I just prefer not to have global/private variables, as they can get destroyed under certain circumstances.

TotallyConfused · Nov 3, 2018

Hello Hitbid

I’m fairly new to VBA so there might be a problem with this, but I did test it using three different sheets, and it seemed to work okay. One advantage I can see is that if you ever needed to combine data from more than three sheets, the only change that would be needed would be to increase the loop counter ‘X’. I can’t take credit for this because all I have done was to modify the code that Fluff wrote.

Code:

Function B_Row1(Sht As String)
  B_Row1 = Sheets(Sht).Cells(Rows.Count, 1).End(xlUp).Row
End Function

Function B_Row2(Sht As String)
  B_Row2 = Sheets(Sht).Cells(Rows.Count, 1).End(xlUp).Offset(1).Row
End Function

Sub Master_Finder()
  Application.ScreenUpdating = False
    Dim X As Integer
For X = 0 To 2
  With Sheets("Sheet" & X + 1)
    .Range(.Cells(1, 1), .Cells(B_Row1(.Name), 2)).Copy Sheets("Test").Cells(B_Row2("Test"), 1)
  End With
Next
Sheets("Test").Activate
ActiveWindow.ScrollRow = 1
Application.ScreenUpdating = True
End Sub

TotallyConfused

TotallyConfused · Nov 3, 2018

Hello Hitbid Update to my posting #5

In reference to my posting #5 , I discovered it can be simplified it even further. Everything is the same as in posting #5 EXCEPT for the following two lines.

Code:

For X = [COLOR=#ff0000]0 To 2[/COLOR]
  With Sheets("Sheet" & X[COLOR=#ff0000] + 1[/COLOR])
 
Which should be changed to read as follows:
 
For X = 1 To [COLOR=#ff0000]3[/COLOR]
  With Sheets("Sheet" & X)

TotallyConfused

hitbid · Nov 3, 2018

How do I update this line here:

Code:

Application.ScreenUpdating = False
With Sheets("sheet1")
   .Range(.Cells(1, 1), .Cells(B_Row1(.Name), 2)).Copy Sheets("Test").Cells(2, 1) [COLOR=#333333]End With
[/COLOR]

Really this line in particular: Cells(B_Row1(.Name), 2)).Copy
How do I format this if my sheet tab is named "IB"? I can't figure out how to translate the .name area.

MARK858 · Nov 3, 2018

If you mean the sheet you are copying from....

Code:

    Application.ScreenUpdating = False
    With Sheets([COLOR="#FF0000"]"IB"[/COLOR])
        .Range(.Cells(1, 1), .Cells(B_Row1(.Name), 2)).Copy Sheets("Test").Cells(2, 1)
    End With

Question about optimizing simple code

hitbid

Board Regular

Excel Facts

Fluff

MrExcel MVP, Moderator

hitbid

Board Regular

Fluff

MrExcel MVP, Moderator

TotallyConfused

Board Regular

TotallyConfused

Board Regular

hitbid

Board Regular

MARK858

MrExcel MVP

Similar threads

Forum statistics

Share this page

Question about optimizing simple code

Board Regular

Excel Facts

MrExcel MVP, Moderator

Board Regular

MrExcel MVP, Moderator

Board Regular

Board Regular

Board Regular

MrExcel MVP

Similar threads

Forum statistics

Share this page

We've detected that you are using an adblocker.

Which adblocker are you using?

Disable AdBlock

Disable AdBlock Plus

Disable uBlock Origin

Disable uBlock