Return to Answer

• 1

• 7.3k
• 1
• 23
• 60

• 7.3k
• 1
• 23
• 60

I don't think there is a StringBuilder() class in VBA, only some tricks using Mid.

Const DELIMIT As String = "|", COLSPAN As Long = 2
Dim HEADER As String

This is a little confusing, UPPERCASE should indicate a constant, which is does with DELIMIT - but Header is not (cannot) be a constant. And that leaves me without a Dim or a Const for COLSPAN. Try to be a little more consistent with that - it will be much easier to tell what variables are what.

Dim ASU as Boolean
ASU = Application.ScreenUpdating
Application.ScreenUpdating = False
Application.ScreenUpdating = ASU

Now, I know ASU can't be a constant. Maybe screenIsUpdating? But then again, I think using a variable to store this is overkill unless you are trying to save the settings of the user - which you aren't

Dim screenIsUpdating as Boolean
screenIsUpdating = Application.ScreenUpdating
Application.ScreenUpdating = False
Application.ScreenUpdating = screenIsUpdating

This way you store the user's settings, but still turn it off for your procedure.

These variables could use better names, even if i and j -

Dim Rw As Long, Col As Long
Dim Ws As Worksheet

Ws works, but I don't recommend it, it will start to look pretty messy once you have a lot going on. Also, local variables should start with a lowercase letter Standard VBA naming conventions.

Dim tblVisibility As Range, rngFormulas As Range, rngConstants As Range

I see tblVisibility and think "oh, must be a boolean" - but it's a range. And rngFormulas and rngConstants seem to have the same issue, which is why they are prefixed with rng - yeah?

tableRange
formulaRange
constantRange

But, what is constantRange? If it's constant, it doesn't need a range.

Cells(Rw, Col).Value2 = Value

You did a good job qualifying most things, but this Cells isn't qualified - it should be inputCell.Parent.Cells - or just give that target sheet a variable.

If MsgBox(Msg, vbYesNo + vbCritical, "Your attention please!") = vbNo Then End

Here's an End again, try to avoid those. Also I think Msg (as well as some other fixed strings) could be a Const.

Rw = inputCell.Row + 1
Col = inputCell.Column
Dim Value As String
For Each Ws In ActiveWorkbook.Worksheets
 Value = Join(Array(Ws.Name, Ws.Visible), DELIMIT)
 Cells(Rw, Col).Value2 = Value
 Rw = Rw + 1
Next

This loop is pretty confusing to me. You are iterating up the rows, but have a loop for the sheets?

For index = 1 to Thisworkbook.Worksheets.Count
 targetSheet.Cells(index+1,tableColumn) = Join(Array(Worksheets(index).Name,Worksheets.Visible), DELIMITER)
Next

But, for that Join string, I would probably do it a different way -

Dim index As Long
Dim tableArray() As String
Dim sheetCount As Long
sheetCount = ThisWorkbook.Worksheets.Count
ReDim tableArray(1 To sheetCount, 1 To 2)
For index = LBound(tableArray) To UBound(tableArray)
 tableArray(index, 1) = ThisWorkbook.Worksheets(index).Name
 tableArray(index, 2) = ThisWorkbook.Worksheets(index).Visible
Next

Arrays are faster and you can just Transpose it into your table range. Or maybe just convert the array into a table.