I agree with the OP:
It seems quite complex [...] a fair
amount of redundant lines present that can tidy up [the] code.
I am making an assumption that the code works - although it is hard to tell. I will also assume the line Find first row of table (las row of sheet as data previously cleared)
is a typo and meant to be a comment.
I hope the OP will understand that I mean this in the nicest way: That current set of code is a mess - a dog's breakfast of spaghetti. I say this so there are no illusions - there is much to learn from this code and I doubt I will cover it all in this post.
Option Explicit
Something I repeat quite often. Always put Option Explicit
at the top of the module. It prevents simple spelling errors resulting in undefined variables, and makes debugging easier because the run-time code usually gives a better error description and may even highlight the bad line of code.
In this example x
is undeclared, even though it is assigned very early in the code.
Indenting
Always properly indent the code. This makes it easier to read and maintain. It also helps you check your code logic, because an unfinished loop or something inside a loop that should be outside will stand out because of the indenting.
Consistent variable use
Check when variables are being used.
- You set
MyRange
but I don't see it being used anywhere.
- You set
Company_Name = Sheets("General").Range("A" & x).Value
at
the beginning of the loop, but when you check
Sheets("General").Range("A" & x).Value <> ""
at the end of the loop
you don't reset the value. You could set Company_Name
before you
enter the loop, and then reset it when you get to the end of the
loop, resulting in a final check of Loop While Company_Name <> ""
which is easier to understand for the general reader.
This concept is about using your variable names to self-comment the code. Good variable names means that the reader understands what the code is doing without any additional comments.
The use of On Error
The spaghetti code from error handling tells me that the code logic and possible errors has not been considered. The current code is too messy to provide any specific advice. Here are some general pointers:
If you think your code is going to fail because something does not exist, then check for its existence before you enter the code. And encapsulate that error check in a function to both isolate from your code and encourage re-use. Two examples of how to check if a Worksheet exists:
Function WSExists1(wsName as String) as Boolean
Dim result as Boolean '<-- Value set by default to False
Dim tempWS as Worksheet
For each tempWS in ThisWorkbook.Worksheets
If result = result OR (tempWS.Name = wsName)
Next tempWS
WSExists1 = result
End Function
Function WSExists2(wsName as String) as Boolean
Dim result as Boolean '<-- Value set by default to False
On Error Resume Next
result = ThisWorkbook.Worksheets(wsName).Range("A1").Address = "A1"
WSExists1 = result
End Function
Now, in your code you can use If WSExists(Company_Name) Then
with great confidence and no requirement for spaghetti GoTo
.
You can create similar 'helper' functions for other areas where you check for errors.
Performance
Hard to tell in the current code, but I suspect that you could dispense with the Excel Table manipulation and use arrays to work with the data. Noting that .DataRange
returns an array of values anyway.
In addition, your first loop (on Company_Name
) could be based on an array that your get from Sheets("General").Range("A4:A" & LastRowTBD).Value
. This will assist with performance.
However, how much is redundant code and where performance tweaks can be built in will have to wait until the simpler tidy up elements have been done.