###Variables
Variables
###Writing to sheet
Writing to sheet
###Variables
###Writing to sheet
Variables
Writing to sheet
###Variables
You did a good job of naming your variables as what they are, but you didn't declare some.
Always turn on Option Explicit
. You can have it automatically by going to Tools -> Options in the VBE and checking the Require Variable Declaration option. This way if you have any variables not defined, the compiler will let you know.
When you don't define your variable, VBA will declare it as a Variant type that can hold any type of data. While this may be more flexible, it adds processing time to your macro as VBA decides or tests for the type. Additionally, since a Variant can be any type of data, you may miss out on valuable troubleshooting information on Type Mismatch
Also, you can do better with the worksheets. Worksheets have a CodeName
property - View Properties window (F4) and the (Name)
field (the one at the top) can be used as the worksheet name. This way you can avoid Sheets("Import")
and instead just use Import
.
You also ran into the pitfall of VBA naming style. Standard VBA naming conventions have camelCase
for local variables and PascalCase
for other variables and names.
You are missing a function Find_End
which I assume just find the last row. So it would be something like this
Dim inspectionLastRow As Long
inspectionLastRow = UniqueSheet.Cells(Rows.Count, 1).End(xlUp).Row
Now you have your loops. You have an i
loop, a x
loop and a Z
loop. In this case I would give them more specific names -
For inspectionIndex = 2 To inspectionLastRow
For importIndex = 2 To importLastRow
for uniqueIndex = 2 to uniqueLastRow
For targetColumn = 0 To 7
But with the last one, you are using it as a column so you'd be better off adjusting the array than the column -
For z = 1 To 8
Out_Sheet.Cells(i + 2, z).Value = Imp_Arr(i, z - 1)
When you have a string like this -
Imp_Arr(Unique_Sheet.Cells(x, 2) - 1, 3) = "MANUAL INPUT REQUIRED"
I find it better to get that out of the way up top
Const INPUT_REQUIRED As String = "MANUAL IMPORT REQUIRED"
You have a lot of comments. Comments - "code tell you how, comments tell you why". The code should speak for itself, if it needs a comment, it might need to be made more clear. If not, the comment should describe why you're doing something rather than how you're doing it. Here are a few reasons to avoid comments all together.
###Writing to sheet
You have this chunk of code
Imp_Arr(Unique_Sheet.Cells(x, 2) - 1, 0) = Imp_Sheet.Cells(i, 3) Imp_Arr(Unique_Sheet.Cells(x, 2) - 1, 1) = Imp_Sheet.Cells(i, 7) Imp_Arr(Unique_Sheet.Cells(x, 2) - 1, 2) = Imp_Sheet.Cells(i, 8) Imp_Arr(Unique_Sheet.Cells(x, 2) - 1, 3) = "MANUAL INPUT REQUIRED" Imp_Arr(Unique_Sheet.Cells(x, 2) - 1, 4) = Imp_Sheet.Cells(i, 6) Imp_Arr(Unique_Sheet.Cells(x, 2) - 1, 5) = Imp_Sheet.Cells(i, 4) Imp_Arr(Unique_Sheet.Cells(x, 2) - 1, 6) = Imp_Sheet.Cells(i, 5)
That is pretty difficult to follow, yeah? Rearrange your array and write it all at once -
Imp_Sheet.Range(.Cells(i, 3), .Cells(i, 8)) = Imp_Arr
Or pull out dimensions of the array into new arrays to put them on the sheet.
This If
block doesn't need to be a block
If (Unique_Sheet.Cells(i, 1).Value = "") Then Exit For End If
If IsEmpty(Unique_Sheet.Cells(i, 1)) Then Exit For