Skip to main content
deleted 3 characters in body
Source Link
AJD
  • 2.3k
  • 1
  • 9
  • 18
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
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
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
        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
Source Link
AJD
  • 2.3k
  • 1
  • 9
  • 18

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.