Copy, Paste and Format 2.0

Question

This is a follow up with revised code, see the original question and subsequent answer in the following link (Copy, Paste And Format)

This is a full version of all three "Macros" or subs within the workbook that I'm currently working with. I hope to use much of this as a reference in future automation to many other aspects of the work day that will make life for many of my work colleagues quicker and easier! Huge thanks to @Zak for a great answer, as he suggested this is the current revised code. I'm sure that there are still many things I could change and am still open to suggestions, though by and large this works for my purpose and is plenty quick now, running in under a seconds one after another as each "macro" is not required in all reports (this being an internal non-coding matter). I am currently mainly wondering of any bad habits glaringly obvious to the eye. As always all constructive criticism and critiques are welcome, though don't feel you have to fundamentally change my code as it is acceptable as of now for my use of it.

Sub SORT()
'/Sam Buford
' SORT Macro
' 2016年05月23日
 Application.ScreenUpdating = False
 Application.EnableEvents = False
 Application.Calculation = xlManual
'/Macro recorded code follows
Range("A:A,B:B,D:D").Select
 Range("D1").Activate
 Range("A:A,B:B,D:D,I:I,J:J,K:K,L:L").Select
 Range("L1").Activate
 Range("A:A,B:B,D:D,I:I,J:J,K:K,L:L,N:N,O:O,P:P,Q:Q,R:R,S:S").Select
 Range("S1").Activate
 Selection.Delete shift:=xlToLeft
 Application.ScreenUpdating = True
 Application.EnableEvents = True
 Application.Calculation = xlAutomatic
CleanExit:
 Exit Sub
CleanFail:
 '/Resets the Application settings, *then* raises the error
 On Error GoTo 0
 Application.ScreenUpdating = True
 Application.EnableEvents = True
 Application.Calculation = xlAutomatic
 Err.Raise (Err.Number)
End Sub
Sub Paste()
'/Paste Macro
' 2016年05月23日
On Error GoTo CleanFail
 Application.ScreenUpdating = False
 Application.EnableEvents = False
 Application.Calculation = xlManual
Dim sht1 As Worksheet
Dim sht2 As Worksheet
Dim LastRow As Long
Dim LastRow2 As Long
Dim LastColumn As Long
Dim StartCell1 As Range
Dim StartCell2 As Range
Dim rng1 As Range
Dim rng2 As Range
Set sht1 = GetWSFromCodeName("Sheet10")
Debug.Print sht1.Name
Set sht2 = GetWSFromCodeName("Sheet8")
Debug.Print sht2.Name
Set StartCell1 = Range("A2")
Set StartCell2 = Range("B2")
'Find Last Row and Column
 LastRow = sht1.Cells(sht1.Rows.Count, StartCell1.Column).End(xlUp).Row
 LastColumn = sht1.Cells(StartCell1.Row, sht1.Columns.Count).End(xlToLeft).Column
 LastRow2 = sht2.Cells(sht2.Rows.Count, StartCell1.Column).End(xlUp).Row
'Select Range And Copy into Final Formula Sheet
 sht1.Range(StartCell1, sht1.Cells(LastRow, LastColumn)).Copy Destination:=sht2.Cells(LastRow2 + 1, 2)
'Convert Text in Column C of Final Formula Sheet to Numbers to Allow Advisor Code to Apply
 Set rng1 = Range(sht2.Cells(LastRow2, 3), sht2.Cells(LastRow2 + LastRow - 1, 3))
 With rng1
 .NumberFormat = "0"
 .Value = .Value
 End With
'Copy Advisor Function down to meet with new Pasted in Data
 With sht2
 Set rng2 = .Cells(LastRow2, 1)
 End With
 With rng2
 .Copy Destination:=Range(sht2.Cells(LastRow2, 1), sht2.Cells(LastRow2 + LastRow - 1, 1))
 End With
 Application.ScreenUpdating = True
 Application.EnableEvents = True
 Application.Calculation = xlAutomatic
CleanExit:
 Exit Sub
CleanFail:
 '/Resets the Application settings, *then* raises the error
 On Error GoTo 0
 Application.ScreenUpdating = True
 Application.EnableEvents = True
 Application.Calculation = xlAutomatic
 Err.Raise (Err.Number)
End Sub
Public Sub ReplaceBlanksTeamID()
'/Fill Blank Team ID's Macro
'2016-05-23
 On Error GoTo CleanFail
 Application.ScreenUpdating = False
 Application.EnableEvents = False
 Application.Calculation = xlManual
Dim sht1 As Worksheet
Dim sht2 As Worksheet
Dim LastRow As Long
Dim StartCell1 As Range
Set sht1 = GetWSFromCodeName("Sheet10")
Debug.Print sht1.Name
Set sht2 = GetWSFromCodeName("Sheet8")
Debug.Print sht2.Name
Set StartCell1 = Range("A2")
'Find Last Row
 LastRow = sht1.Cells(sht1.Rows.Count, StartCell1.Column).End(xlUp).Row
'Select Range
sht1.Range(StartCell1, sht1.Cells(LastRow, 2)).Select
On Error Resume Next
sht1.Range(sht1.Cells(2, 2), sht1.Cells(LastRow, 2)).Select
With Selection.SpecialCells(xlCellTypeBlanks)
.FormulaR1C1 = "=R[+1]C"
.Value = .Value
End With
Application.ScreenUpdating = True
 Application.EnableEvents = True
 Application.Calculation = xlAutomatic
CleanExit:
 Exit Sub
CleanFail:
 '/Resets the Application settings, *then* raises the error
 On Error GoTo 0
 Application.ScreenUpdating = True
 Application.EnableEvents = True
 Application.Calculation = xlAutomatic
 Err.Raise (Err.Number)
End Sub
 '/This Function allows the worksheet name to change in the workbook as it allows the
 'user to set Worksheets to codename variables. By using this function the user can input a
 'codename for a worksheet and the function will call the worksheet name of the corresponding
 'codename, allowing the user to set worksheet variables to codenames without losing
 'functionality usually associated with such variables.
 '2016-05-23
Public Function GetWSFromCodeName(CodeName As String) As Worksheet
 On Error GoTo CleanFail
 Application.ScreenUpdating = False
 Application.EnableEvents = False
 Application.Calculation = xlManual
 Dim WS As Worksheet
 For Each WS In ThisWorkbook.Worksheets
 If StrComp(WS.CodeName, CodeName, vbTextCompare) = 0 Then
 Set GetWSFromCodeName = WS
 Exit Function
 End If
 Next WS
Application.ScreenUpdating = True
 Application.EnableEvents = True
 Application.Calculation = xlAutomatic
CleanExit:
 Exit Function
CleanFail:
 '/Resets the Application settings, *then* raises the error
 On Error GoTo 0
 Application.ScreenUpdating = True
 Application.EnableEvents = True
 Application.Calculation = xlAutomatic
 Err.Raise (Err.Number)
End Function

• Sam Buford: Thanks to all that helped with this! I have learned some better coding practices, learned to rethink why i implemented certain things and if there is a better way, as well as learned some info on how Excel and Worksheets interact. This has greatly improved my code and made me very thankful that Code Review exists. Thanks Again!

Answer 1

@Raystafarian beat me to it, but it cannot be said too many times: avoid Select and Activate, keep that for macro-recorder generated code!

Range("A:A,B:B,D:D").Select
Range("D1").Activate
Range("A:A,B:B,D:D,I:I,J:J,K:K,L:L").Select
Range("L1").Activate
Range("A:A,B:B,D:D,I:I,J:J,K:K,L:L,N:N,O:O,P:P,Q:Q,R:R,S:S").Select
Range("S1").Activate
Selection.Delete shift:=xlToLeft

Most of this code is useless, you're making a selection, and then making another selection, and then making another selection... and then making another selection, and deleting it.

This would be equivalent:

Range("S1").Select
Selection.Delete shift:=xlToLeft

And this would be a little better:

Range("S1").Delete shift:=xlToLeft

Except Range is implicitly referring to the active worksheet. Chances are, that's not what you want to do: you want to be referring to a specific sheet, e.g. Sheet2. Then *qualifytheRange` function with it:

Sheet2.Range("S1").Delete shift:=xlToLeft

---

This is very troubling:

Public Function GetWSFromCodeName(CodeName As String) As Worksheet

The code name property of a worksheet is used to determine what identifier to use in code to refer to a global Worksheet object that refers to a specific worksheet. If you know the CodeName of a worksheet, you already know what that worksheet is.

GetWSFromCodeName("Sheet42")

Returns the same object as

Sheet42

VBA already creates an object named Sheet42 for you to use; there's no need to create another object to refer to the same thing.

---

You're repeating this chunk quite often:

Application.ScreenUpdating = {Boolean}
Application.EnableEvents = {Boolean}
Application.Calculation = {xlCalculation}

Why not extract it into its own parameterized function?

---

Your indentation is hard to follow.

With rng1
 .NumberFormat = "0"
 .Value = .Value
 End With

Try to line up Foo...End Foo statements:

With rng1
 .NumberFormat = "0"
 .Value = .Value
End With

It makes it much easier to see what starts and ends where.

Sometimes indentation is simply inexistent:

With Selection.SpecialCells(xlCellTypeBlanks)
.FormulaR1C1 = "=R[+1]C"
.Value = .Value
End With

Some VBE add-ins offer an indenter tool to ensure consistent indentation throughout a project. I suggest you take a look at the latest MZ-Tools and/or Rubberduck - note/disclaimer: I'm heavily involved with Rubberduck.

• Sam Buford: Thanks @Mat's Mug! this is very useful info that will surely aid me in future eneavors of coding. I'm beginning to love SO and all its sister websites for its wonderful helpful community. I will look into all these changes time allowing and get back to this thread with my fixes and any issues I have. Thanks again to the SO community for this wonderful first day experience!
• Mathieu Guindon: @SamBuford cough this is the CR community ;-) ...if you run into specific issues, you should research first and ask on SO as a last resort; if you want further feedback I'd suggest making a new post, as changing the code in the question to incorporate feedback from answers is very much frowned upon on this site, and will be rolled back. I'd suggest you let a couple of days go by, to give more people a chance to point something out in this code, which has quite much to be said about. Cheers! =)
• Sam Buford: Thanks for your advice and ah i see your point there. CR is Stack exchange sister website just as SO, SO is not the main. Any who, I will leave this on and let the ideas build so as to hopefully learn some great practices as much of coding has been self taught except for a simple excel class taken last year at college.
• Sam Buford: Hmm. Yes I do understand renaming WS doesn't rename the CodeName, this is why i built the function in the first place. What I didn't know was that you could use the cell references with CodeNames. I should then always use the CodeNames yes? Never a need to use WS names as any changes to them within the Workboob which is easily changed by people unfamiliar with Excel would result in broken VBA Code?
• Kaz: The thing to realise is that, if your sheet has a *codename* of sheet42 and a *name* of "Some Sheet", then Sheets("Some Sheet") is the same object as sheet42, which are both worksheet objects and so can be used like so: sheet42.Range().NumberFormat.... etc.

Answer 2

The low hanging fruit this time is the use of .select and .activate.

Be sure to avoid things like .Select - it just slows the code down by needing to fiddle with the spreadsheet while doing everything else behind the scenes. There's a good question on StackOverflow addressing this - https://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba-macros .

This section right here -

Range("A:A,B:B,D:D").Select
Range("D1").Activate
Range("A:A,B:B,D:D,I:I,J:J,K:K,L:L").Select
Range("L1").Activate
Range("A:A,B:B,D:D,I:I,J:J,K:K,L:L,N:N,O:O,P:P,Q:Q,R:R,S:S").Select
Range("S1").Activate
Selection.Delete shift:=xlToLeft

What does it do? It deletes cell S1. That's all. It can be consolidated into one line -

Range("S1").Delete shift:=xlToLeft

---

Now this chunk of code

'Select Range
sht1.Range(StartCell1, sht1.Cells(LastRow, 2)).Select
On Error Resume Next
sht1.Range(sht1.Cells(2, 2), sht1.Cells(LastRow, 2)).Select
With Selection.SpecialCells(xlCellTypeBlanks)
.FormulaR1C1 = "=R[+1]C"
.Value = .Value
End With

You have On Error Resume Next but I don't see a For and you have an error handler. What's the goal here? To fill in blanks? You can use a loop and IsEmpty or IsBlank instead.

For Each c In sht1.Range(sht1.Cells(2, 2), sht1.Cells(LastRow, 2))
 If IsEmpty(c) Then 'do something
Next c

• Sam Buford: Thanks! I checked that article out in great detail during my learning of this and in a similar question I asked on the main SO site. That is definitely something easy I can update and will ensure good practices moving forward.
• Sam Buford: That code simply sorts raw imported data from a very nastily formatted file and removes unnecessary columns, 11 columns to be exact that are not needed in the following worksheet. that was a recorded macro-a quick fix if you will- about 4 months ago that I made very quickly. Now that I have more time to dedicate to this, hopefully i can clean it up a bit. Atleast in practice that is what is occurring in my worksheet