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)

 

[Tupe77]

so, would it be fair to say that the CASE function works best for a long list of potential outcomes with only one else clause?

Yes, but I think you only have two potential outcomes, but more exceptions (condition).

 

[Keebler]

Yes, but I think you only have two potential outcomes, but more exceptions (condition).

true
i have 3 sheets (INDEX, DATA, AND CURRENT (ACTIVE) SHEET) that i do not want the copied range data to be sent to
and several others that i want updated with the updated range

 

[Tupe77]

I lied!
The CASE function works in that case. My logic went haywire.

 

[Keebler]

lol, hopefully, we are all increasing our own knowledge bases

 

[Joe4]

you are correct, writing a1+b1+c1+d1 is tedious and prone to mistakes, sum(a1:d1) is what I use. to be fair, i dont guess i ever thought about writing it out long form.
but if you had to skip cells then you would have no choice but to write it out long form
a1+c1+e1+g1

That was just one example I was using to illustrate a few key points. The 2 points that I was trying to make is the following:
- In Excel, there are often numerous ways of accomplishing the same task
- Even different methods which appear to be doing the same thing may not work exactly the same, and may behave differently in different scenarios

 

[Keebler]

so in my case
every sheet has the potential to be updated
but like what RoryA said if the sheets are defined as NOT UPDATE
"Case "INDEX", "DATA", "SOME OTHER NAME""
then or "Case Else"
update

which brings me back to question about CASE
can the CASE declarations be referenced as variables?

dim sws as activesheet

"Case "INDEX", "DATA", sws""

 

[Tupe77]

All versions.

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

' ***** 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

' ***** CASE
For Each wsTMP In Worksheets
   Select Case UCase$(wsTMP.Name)
      Case "INDEX", "DATA", "SOME OTHER NAME"
        ' do nothing
      Case Else
        Debug.Print "It's sheet " & UCase(wsTMP.Name) & " turn in loop"
   End Select
Next

' ***** if/then/else?
For Each wsTMP In Worksheets
    If UCase(wsTMP.Name) <> "INDEX" And UCase(wsTMP.Name) <> "DATA" And UCase(wsTMP.Name) <> wsCurrent.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

 

[Keebler]

That was just one example I was using to illustrate a few key points. The 2 points that I was trying to make is the following:
- In Excel, there are often numerous ways of accomplishing the same task
- Even different methods which appear to be doing the same thing may not work exactly the same, and may behave differently in different scenarios

so kinda like MS giving us the rope and several ways to hang ourselves?

 

[Keebler]

All versions.

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

' ***** 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

' ***** CASE
For Each wsTMP In Worksheets
   Select Case UCase$(wsTMP.Name)
      Case "INDEX", "DATA", "SOME OTHER NAME"
        ' do nothing
      Case Else
        Debug.Print "It's sheet " & UCase(wsTMP.Name) & " turn in loop"
   End Select
Next

' ***** if/then/else?
For Each wsTMP In Worksheets
    If UCase(wsTMP.Name) <> "INDEX" And UCase(wsTMP.Name) <> "DATA" And UCase(wsTMP.Name) <> wsCurrent.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

not a programmer?

wow this spells it out pretty well - thank you
i see what you did
use the wsTMP.name as the variable that holds the skipped worksheets
the if/then/else looks messy and could very lead to one getting lost through it

so either the dictionary or the CASE

 

[Joe4]

so kinda like MS giving us the rope and several ways to hang ourselves?

Excel offers you lots of tools.
What you do with them is up to you!