clean code

[Keebler]

background: I have learned so much from this site as well as my own research, but, as you will see, I leanred myself into a corner and would like some direction please.

I have this vba code that I use to sort a long list of titles. the code "works", but I would like a way to clean it up.
so my code defines variables then sorts based on a conglomeration of "lrow", "scol", and "srow" (see "slist")
(lrow is the last row in the column that has data) (scol is the start column) (srow is the start row) and (slist is compiled range)

1) is there a way to combine the column variable and the row variable to produce an end range? ex: (row = 3; column = 17 (Q)) to get range ("Q3") without having to change the "Q3" to match what column range I am working on? (ag3 in this case)

2) is there a way to actually sort a range removing all the blank cells? perhaps faster than the way that I am sorting the column.... row by row by row (very slow when you have over 10k rows)

3)
(in the current rendition, there are over 13k rows that amount to about 8k with the blank cells removed) removing the blank cells as they are copied would take years... ( I have modules that copy the data from various pages, then I go the index page (where the copied lists are to be sorted, etc) and paste the values, then run the sorting module, etc.

Sub SORT_ALBUM() 'SORTS ONLY THE ALBUM COLUMN(S)
Dim lrow As Long
Dim srow As String, scol As String, slist As String
Dim lrow2 As Long
Dim srow2 As String, slist2 As String
Dim CNT As Integer, C As Integer, cnum As Integer

'define variables
lrow = Range("ak1")
scol = "ag"
srow = 3
slist = "ag" & srow & ":ag" & lrow
cnum = Range(scol & 1).Column

'sort
Range(slist).Sort KEY1:=Range("ag3"), _
                     ORDER1:=xlAscending, _
                     Header:=xlNo

'remove blanks
For C = srow To lrow
    If Cells(C, cnum).Value <> "" Then
    Cells(CNT + 1, cnum + 1).Value = Cells(C, cnum).Value
    CNT = CNT + 1
    End If
Next C

'insert 2 rows
Range("ah1:ah2").Insert xlShiftDown

'define new variables
lrow2 = Range("al1") + 1
slist2 = "ah" & srow & ":ah" & lrow2

'move sorted data to correct home
Range(slist2).Copy Range("ag3")
Range(slist2).ClearContents

End Sub

so here is my vba code (this is not the whole module - only the first part that could be copied to other sections) (as I said, this works. it is just not clean, fast, or eligant)

 

[Keebler]

Perhaps I should provide more details
the first five columns are for the raw data (it is where the lists are imported) then from column F (6) on is where the list is broken down and arranged as I want. these columns (currently there are about 230 columns)(1) contain all the formulas and such to arrange the raw lists into more manageable data that can then be imported into my INDEX sheet that contains all the sorted and arranged data.
the first five columns in most of the sheets(2) are the only parts on all the sheets (the ones I need to update when I modify the formulas) that I do not want updated.


(1) not all the columns have any data or formulas, most are empty and provide nothing more than formatting/spacing so i can see how each line of the list is arranged
(2) all the sheets except for the INDEX, DATA, and the worksheet I am modifying


hope this helps clarify what I am doing and why I want to build a code that will save some time...

 

[RoryA]

It sounds to me like it would be easier to amalgamate the data and process it in one place, rather than copying and pasting formulas across all the sheets.

Anyway, I see no reason not to use a loop. You can use a simple Select Case (or If) block to skip [particular names.

 

[Keebler]

It sounds to me like it would be easier to amalgamate the data and process it in one place, rather than copying and pasting formulas across all the sheets.

Anyway, I see no reason not to use a loop. You can use a simple Select Case (or If) block to skip [particular names.

would it be possible to show me how that would work? "to amalgamate the data and process it in one place,"?

 

[Tupe77]

is there a way to list all the worksheets (in this workbook) and select all but 2 (named : "index" and "data") (maybe 3 sheets to include "FMA0101")

Do you mean something like this?
This is a loop for going through the sheets. Contains three exceptions: INDEX, DATA and ActiveSheet.

Sub TS_LoopSheetsWithExceptions()
Dim wsCurrent As Worksheet: Set wsCurrent = ActiveSheet ' Get Working sheet name to skip it.
Dim wsTMP As Worksheet
Dim dict As Object: Set dict = CreateObject("Scripting.Dictionary"): dict.CompareMode = vbTextCompare ' Create Dictionary

' ***** Add the names of the Sheets to be skipped. **************
dict.Add "INDEX", 1
dict.Add "DATA", 1
dict.Add wsCurrent.Name, 1
 
' ***** Go through all the sheets that have not been added to the dictionary.
For Each wsTMP In Worksheets
    If Not dict.Exists(wsTMP.Name) Then
        ' Add your code here START
        Debug.Print "It's sheet " & UCase(wsTMP.Name) & " turn in loop"
        ' Add your code here END
    End If
Next wsTMP
End Sub

My apologies for any quirks, English is not my native language. "So I blame Google translate for all my goofs."

 

[RoryA]

would it be possible to show me how that would work? "to amalgamate the data and process it in one place,"?

It would depend on your workbook (230 columns of formulas is a lot!), but generally I'd use power query to combine all the sheet data into one data set and then do whatever I needed with that one.

 

[Keebler]

Do you mean something like this?
This is a loop for going through the sheets. Contains three exceptions: INDEX, DATA and ActiveSheet.

Sub TS_LoopSheetsWithExceptions()
Dim wsCurrent As Worksheet: Set wsCurrent = ActiveSheet ' Get Working sheet name to skip it.
Dim wsTMP As Worksheet
Dim dict As Object: Set dict = CreateObject("Scripting.Dictionary"): dict.CompareMode = vbTextCompare ' Create Dictionary

' ***** Add the names of the Sheets to be skipped. **************
dict.Add "INDEX", 1
dict.Add "DATA", 1
dict.Add wsCurrent.Name, 1
 
' ***** Go through all the sheets that have not been added to the dictionary.
For Each wsTMP In Worksheets
    If Not dict.Exists(wsTMP.Name) Then
        ' Add your code here START
        Debug.Print "It's sheet " & UCase(wsTMP.Name) & " turn in loop"
        ' Add your code here END
    End If
Next wsTMP
End Sub

My apologies for any quirks, English is not my native language. "So I blame Google translate for all my goofs."

can you please explain to me the addition of the dictionary?
this is a new command or reference to me, so forgive my ignorance...
i understand the loop
but not the debug statement - is that a pop-up message that tells me what sheet it is processing? (if so, how vital is it?)
the insert code lines are self explanatory

thank you, ill give this a whirl... it probably will not be today (Thursday) but I should get back to this Friday... thank you!

 

[Keebler]

It would depend on your workbook (230 columns of formulas is a lot!), but generally I'd use power query to combine all the sheet data into one data set and then do whatever I needed with that one.

RoryA
thank you,
like i mentioned previously. most of the 230 columns is blank. they are there to make the formatting easier to read.
I'll have to look more into the power query and data set. thank you. i have to admit that sounds like a huge time saver over having potentially hundreds of sheets with only the first 5 columns being different.

 

[RoryA]

A Dictionary seems like overkill to me:

For Each wsTMP In Worksheets
   Select Case UCase$(wsTMP.Name)
      Case "INDEX", "DATA", "SOME OTHER NAME"
      ' do nothing
      Case Else
      ' run your code here
   End Select
Next

 

[Keebler]

A Dictionary seems like overkill to me:

For Each wsTMP In Worksheets
   Select Case UCase$(wsTMP.Name)
      Case "INDEX", "DATA", "SOME OTHER NAME"
      ' do nothing
      Case Else
      ' run your code here
   End Select
Next

this makes a lot of sense to me
can the "some other name" be a activesheet variable? like
dim aws as activesheet
or do i need to define aws as a worksheet prior?
dim aws as worksheet, aws as activesheet

 

[Tupe77]

A Dictionary seems like overkill to me:

For Each wsTMP In Worksheets
   Select Case UCase$(wsTMP.Name)
      Case "INDEX", "DATA", "SOME OTHER NAME"
      ' do nothing
      Case Else
      ' run your code here
   End Select
Next

As far as I understand, CASE doesn't work that way?
The comma is read as OR, so whenever the loop meets the INDEX sheet, the Case "INDEX", "DATA", "SOME OTHER NAME" is executed
It doesn't get to CASE Else - part after that.