Your code has tremendously improved since the first time I saw it first time I saw it - good job!
Your code has tremendously improved since the first time I saw it - good job!
Your code has tremendously improved since the first time I saw it - good job!
Your code has tremendously improved since the first time I saw it - good job!
This particular line is hard to parse:
wsImport.Range(source.Offset(1), wsImport.Cells(Rows.Count, source.Column).End(xlUp)).Copy _
wsMain.Cells(Rows.Count, dest.Column).End(xlUp)(2)
Literally: it's crashing the rubberduck parser!
You could introduce a local variable here:
Dim target As Range
Set target = wsMain.Cells(Rows.Count, dest.Column).End(xlUp)
wsImport.Range(source.Offset(1), wsImport.Cells(Rows.Count, source.Column).End(xlUp)) _
.Copy target(RowIndex:=2)
Notice how the line continuation is placed so that no instruction is split; rubberduck doesn't care about line continuations, but it's much easier for the human eye to see what function calls return the arguments for which procedure if you don't split an instruction between the name of a procedure and its arguments - vertically lining up .Range
and .Copy
also make it clearer that .Copy
operates on the result of .Range
.
Dim wsImport As Worksheet, wsMain As Worksheet
Is this really buying you anything? Multiple declarations on a single line make it harder to locate declarations at a glance. Compare to:
Dim wsImport As Worksheet
Dim wsMain As Worksheet
My eye sees Dim
, my brain sees "variable declaration here" - two Dim
s, two variables. And I read the variable name at pretty much the exact same millisecond as the one I notice the Dim
statement, because I don't need to mentally scroll horizontally and locate the comma. Two variables isn't too bad, but more than that would be problematic. Better avoid multiple declarations on a single line.
There's a redundant reference to wsMain
in this With
block:
With wsMain
.Columns("A:AO").AutoFit
.Cells.ClearFormats
.Rows(1).Font.Bold = True
.Cells.Font.Name = "Georgia"
.Cells.Font.Color = RGB(0, 0, 225)
.Cells.Resize(wsMain.Rows.Count - 1).Offset(1).Interior.Color = RGB(216, 228, 188)
End With
See it? Right here:
.Cells.Resize(wsMain.Rows.Count - 1).Offset(1).Interior.Color = RGB(216, 228, 188)
Could be
.Cells.Resize(.Rows.Count - 1).Offset(1).Interior.Color = RGB(216, 228, 188)
The extraneous empty lines before End With
should be removed, too.
Your indentation isn't consistent.
Dim ws As Worksheet
For Each ws In Worksheets
ws.Select
ActiveWindow.Zoom = 85
Next ws
The only thing that should be at the same indentation level as Public Sub
/End Sub
, is line labels (which the VBE forces to start at column 1 anyway).