clean code

Keebler

Board Regular
Joined
Dec 1, 2021
Messages
167
Office Version
  1. 2021
Platform
  1. Windows
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.

VBA Code:
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)
 
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.
would CASE work if it was before the if/then loop?
defining it as a string or long
then when the loop encounters an item in the case variable it would skip it - yes?
 
Upvote 0

Excel Facts

How to fill five years of quarters?
Type 1Q-2023 in a cell. Grab the fill handle and drag down or right. After 4Q-2023, Excel will jump to 1Q-2024. Dash can be any character.
A Dictionary seems like overkill to me:

VBA Code:
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
The dictionary itself is definitely overkill, but it's easy to manage and doesn't require changes to the code when adding or subtracting exceptions. (Here exceptions are added manually to the code, but usually when they are read from somewhere.)
I mean compared to "IF ... OR" or nested loops (Sheets and exceptions)
 
Upvote 0
Tupe77,
so is the dictionary a temporary "book shelf" whereas one might store a bunch of variables?
then is the dictionary wiped clean when the vba ends?
if it is a bookshelf as it suggests, is it fair to assume that this virtual bookshelf uses a bunch of resources to store said bookshelf? or are they just place holders and dont actually store the "books"?
 
Upvote 0
would CASE work if it was before the if/then loop?
defining it as a string or long
then when the loop encounters an item in the case variable it would skip it - yes?
At the moment, I myself can't figure out how to make CASE work logically in this matter. It definitely doesn't mean that someone here doesn't know how to write it to work.

WorkSheetList -> CASE -> Do something and delete used Case from WorkSheetList would work but is a bit complicated
 
Upvote 0
according to MS
"

Description​


The Microsoft Excel CASE statement has the functionality of an IF-THEN-ELSE statement.

The CASE statement is a built-in function in Excel that is categorized as a Logical Function. It can be used as a VBA function (VBA) in Excel. As a VBA function, you can use this function in macro code that is entered through the Microsoft Visual Basic Editor.
"
VBA Code:
Select Case LRegion
   Case "N"
      LRegionName = "North"
   Case "S"
      LRegionName = "South"
   Case "E"
      LRegionName = "East"
   Case "W"
      LRegionName = "West"
   End Select
 
Upvote 0
im not suggesting that any of it is incorrect, i want to learn.
my questions arise from a learning aspect, not to condemn
 
Upvote 0
so, i have to ask
if select/case is the same as if/then/else
why not use if/then/else?
 
Upvote 0
A dictionary contains a key and a value.
The key must be unique, otherwise it will throw an exception.
It prevents the creation of duplicates, but it also enables the possibility to check if the key exists. In other methods, the existence of the value must be checked by looping through the list, array, etc.
I don't know if this clears anything up?
 
Upvote 0
actually yes
thank you

i can see where that can be overkill
according to MS
VBA Code:
Dim d                   'Create a variable
Set d = CreateObject("Scripting.Dictionary")
d.Add "a", "Athens"     'Add some keys and items
d.Add "b", "Belgrade"
d.Add "c", "Cairo"

it could potentially be a huge list

can this list then be importable or reference a ws range?
 
Upvote 0
if it could be importable, then that could save a bunch of time - correct?
 
Upvote 0

Forum statistics

Threads
1,215,872
Messages
6,127,428
Members
449,382
Latest member
DonnaRisso

We've detected that you are using an adblocker.

We have a great community of people providing Excel help here, but the hosting costs are enormous. You can help keep this site running by allowing ads on MrExcel.com.
Allow Ads at MrExcel

Which adblocker are you using?

Disable AdBlock

Follow these easy steps to disable AdBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the icon in the browser’s toolbar.
2)Click on the "Pause on this site" option.
Go back

Disable AdBlock Plus

Follow these easy steps to disable AdBlock Plus

1)Click on the icon in the browser’s toolbar.
2)Click on the toggle to disable it for "mrexcel.com".
Go back

Disable uBlock Origin

Follow these easy steps to disable uBlock Origin

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back

Disable uBlock

Follow these easy steps to disable uBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back
Back
Top