Skip to main content
Code Review

Return to Revisions

3 of 3
replaced http://stackoverflow.com/ with https://stackoverflow.com/

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
Raystafarian
  • 7.3k
  • 1
  • 23
  • 60
default

AltStyle によって変換されたページ (->オリジナル) /