Housing a summary of information in a data file

Question

So far it is working great, but I was wondering if I could optimize a little since I have nearly 30 variables in code of only 100 lines.

Since I'm not a coder, I code based on how I would be doing it manually:

1. I have a main file that will, when complete, house a summary of all the information in my data file. I assign variables for the Main file and the data file and open the data file with the getopen code.

2. I have 4 variables for the specific columns I will be referencing (will probably need to add more before the end of this code). I manually assigned a letter because the columns will rarely if ever change. I then put a formula in to assign a variable to the column #.

3. I have variables for the last column and last row.

4. I insert 3 lines which will be populated with data and assign variables for the column letter, and # and populate a header for those columns.

5. I set variables for each range of those columns.

6. I sort the data based on 2 columns.

7. I loop through the data and populate the columns I inserted earlier.

Obviously my methodology to create this, translating how I would do it manually, isn't ideal, but are there shortcuts for any of this? Everything works fine right now, it just seems like I'm taking the long way. Do I really need to create 3 different variables for the column letter, column # and range of column?

Dim Question1 As String '/ is this the current Tab to work on?

Dim TT As String '/ Column Letter for Total Time in seconds
Dim TTN As Long '/ Column # for Total time in seconds
Dim TTR As Range '/ Column as Range Total time

Dim CN As String '/ Column Letter for Call Number
Dim CNN As Long '/ Column # for Call Number
Dim CNR As Range '/ Column as Range Call Number

Dim TY As String '/ Column letter for Type
Dim TYN As Long '/ Column # for Type
Dim TYR As Range '/ Column as range Type

Dim NC As String '/ Column letter for Number called
Dim NCN As Long '/ Column # for Number called
Dim NCR As Range '/ Column as range for Number called

Dim ER As Range '/ Range of data

Dim LRow As Long '/ Finding last Row of data
Dim LCol As Long '/ Finding last Column of data

Dim Uni As String '/ Unique Column
Dim UniN As Long '/ Column #
Dim UniR As Range '/ Column as range

Dim Dur As String '/ Duration Column
Dim DurN As Long '/ Column #
Dim DurR As Range '/ Column as range

Dim LastTFN As String '/ Last 4 digits Column
Dim LastTFNN As Long '/ Column #
Dim LastTFNR As Range '/ Column as range

Dim TyLo As Long '/ Type Loop

Set InFi = ActiveWorkbook.ActiveSheet

    Question1 = MsgBox("Are you allocating data for the week of " & Replace(ActiveSheet.Name, ".", "/") & "?", vbYesNo, "")

    If Question1 = vbYes Then
        Set InFi = ActiveWorkbook.ActiveSheet
    ElseIf Question1 = vbNo Then
        MsgBox ("Goto the current week and re-run")
        Exit Sub
    End If


    FileName = Application.GetOpenFilename

        If FileName <> False Then
            Workbooks.Open FileName:=FileName
            End If

Set CaFi = ActiveWorkbook.Sheets("Calls")
    CaFi.Activate
    Cells(1, 1).Select

    TT = "H" 
    TTN = Asc(TT) - 64 'Numerical value of TT

    CN = "C" 
    CNN = Asc(CN) - 64 'Numerical value of CN

    TY = "M"
    TYN = Asc(TY) - 64 'Numerical value of TY

    NC = "I"
    NCN = Asc(NC) - 64 'Numerical value of NC

    LRow = Cells(Rows.count, 1).End(xlUp).Row
    LCol = Cells(1, Columns.count).End(xlToLeft).Column

    Columns(TYN + 1).EntireColumn.Insert
        Uni = "N"
        UniN = Asc(Uni) - 64
        Cells(1, UniN).Value = "Unique or Duplicate"
    Columns(UniN + 1).EntireColumn.Insert
        Dur = "O"
        DurN = Asc(Dur) - 64
        Cells(1, DurN).Value = "Call Durations"
    Columns(DurN + 1).EntireColumn.Insert
        LastTFN = "P"
        LastTFNN = Asc(LastTFN) - 64
        Cells(1, LastTFNN).Value = "Last 4 TFN"


    With CaFi
        Set TTR = .Range(.Cells(1, TTN), .Cells(1, TTN).End(xlDown))
        Set CNR = .Range(.Cells(1, CNN), .Cells(1, CNN).End(xlDown))
        Set TYR = .Range(.Cells(1, TYN), .Cells(1, TYN).End(xlDown))
        Set NCR = .Range(.Cells(1, NCN), .Cells(1, NCN).End(xlDown))
        Set ER = .Range(.Cells(1, 1), .Cells(LRow, LCol))
        Set UniR = .Range(.Cells(1, UniN), .Cells(1, UniN).End(xlDown))
        Set DurR = .Range(.Cells(1, DurN), .Cells(1, DurN).End(xlDown))
        Set LastTFNR = .Range(.Cells(1, LastTFNN), .Cells(1, LastTFNN).End(xlDown))

    End With

    With CaFi.Sort
        .SortFields.Clear
        .SortFields.Add key:=CNR, SortOn:=xlSortOnValues, Order:=xlAscending, DataOption:=xlSortTextAsNumbers
        .SortFields.Add key:=TTR, SortOn:=xlSortOnValues, Order:=xlDescending, DataOption:=xlSortTextAsNumbers

        .SetRange ER
        .Header = xlYes
        .MatchCase = False
        .Orientation = xlTopToBottom
        .SortMethod = xlPinYin
        .Apply
    End With

For TyLo = 2 To LRow

    '/ Unique vs Duplicate
    If Cells(TyLo, CNN).Value <> Cells(TyLo - 1, CNN).Value Then
        Cells(TyLo, TYN + 1).Value = "Unique"
    Else
        Cells(TyLo, TYN + 1).Value = "Duplicate"
    End If

    '/ Total Time 
    If Cells(TyLo, TTN).Value > 1 And Cells(TyLo, TTN).Value < 30 Then
        Cells(TyLo, TYN + 2).Value = ">1"
    ElseIf Cells(TyLo, TTN).Value > 30 And Cells(TyLo, TTN).Value < 60 Then
        Cells(TyLo, TYN + 2).Value = ">30"
    ElseIf Cells(TyLo, TTN).Value > 60 And Cells(TyLo, TTN).Value < 600 Then
        Cells(TyLo, TYN + 2).Value = ">60"
    ElseIf Cells(TyLo, TTN).Value > 600 Then
        Cells(TyLo, TYN + 2).Value = ">600"
    Else: Cells(TyLo, TYN + 2).Value = "Missed Call"
    End If

    Cells(TyLo, TYN + 3).Value = Right(Cells(TyLo, NCN).Value, 4)

Next

End Sub

Answer 1

Manually assigning column letters

I have 4 variables for the specific columns I will be referencing (will probably need to add more before the end of this code). I manually assigned a letter because the columns will rarely if ever change. I then put a formula in to assign a variable to the column #

If you need the column number, just make the column numbers e.g. -

Const TOTAL_TIME As Long = 8
Const CALL_NUMBER As Long = 3

This saves you from running the Asc and it also makes it more clear when assigning the ranges. Now you set it up top and if you need to change it, just change it there.

So it would be

Const TOTAL_TIME As Long = 8
Const CALL_NUMBER As Long = 3
Const TYPE_COLUMN As Long = 13
Const NUMBER_CALLED As Long = 9
Const UNIQUE_COLUMN As Long = 14
Const DURATION_COLUMN As Long = 15
Const LAST_DIGIT_COLUMN As Long = 16

---

Variables

Always turn on Option Explicit. You can have it automatically by going to Tools -> Options in the VBE and checking the Require Variable Declaration option. This way if you have any variables not defined, the compiler will let you know.

Variables infi, filename and cafi are not declared.

When you don't define your variable, VBA will declare it as a Variant type that can hold any type of data. While this may be more flexible, it adds processing time to your macro as VBA decides or tests for the type. Additionally, since a Variant can be any type of data, you may miss out on valuable troubleshooting information on Type Mismatch

Additionally, your naming could be improved. See how you needed comments next to all your variable declarations?

Dim TT As String '/ Column Letter for Total Time in seconds
Dim TTN As Long '/ Column # for Total time in seconds
Dim TTR As Range '/ Column as Range Total time

Dim CN As String '/ Column Letter for Call Number
Dim CNN As Long '/ Column # for Call Number
Dim CNR As Range '/ Column as Range Call Number

Comments - "code tell you how, comments tell you why". The code should speak for itself, if it needs a comment, it might need to be made more clear. If not, the comment should describe why you're doing something rather than how you're doing it. Here are a few reasons to avoid comments all together.

Use the variable's name to tell us what it does. TTN means nothing to the reader - he/she needs to go back up to look at the comment. Whereas if it was just called totalTimeColumnNumber (for instance) it would be clear.

Error Handling

This line right here

Set cafi = ActiveWorkbook.Sheets("Calls")

needs some error handling. What if it doesn't exist? Furthermore, Worksheets have a CodeName property - View Properties window (F4) and the (Name) field (the one at the top) can be used as the worksheet name. This way you can avoid Sheets("calls") and instead just use Calls.

Select

cafi.Activate
Cells(1, 1).Select

Be sure to avoid things like .Select - it just slows the code down by needing to fiddle with the spreadsheet while doing everything else behind the scenes. There's a good question on StackOverflow addressing this. Essentially, since you're doing everything in the development environment, there's no need to leave it. Just get the data, do your stuff and spit it back out.

Last Row

Set TTR = .Range(.Cells(1, TTN), .Cells(1, TTN).End(xlDown))

You pretty much never want to use xlDown when defining a range. There is a standard way to find lastRow and lastColumn. That post explains why.

Readability

Take this if for instance

If Cells(TyLo, TTN).Value > 1 And Cells(TyLo, TTN).Value < 30 Then
    Cells(TyLo, TYN + 2).Value = ">1"
ElseIf Cells(TyLo, TTN).Value > 30 And Cells(TyLo, TTN).Value < 60 Then
    Cells(TyLo, TYN + 2).Value = ">30"
ElseIf Cells(TyLo, TTN).Value > 60 And Cells(TyLo, TTN).Value < 600 Then
    Cells(TyLo, TYN + 2).Value = ">60"
ElseIf Cells(TyLo, TTN).Value > 600 Then
    Cells(TyLo, TYN + 2).Value = ">600"
Else: Cells(TyLo, TYN + 2).Value = "Missed Call"
End If

Can you look at that and tell what's happening - because I can't. Even just a simple change in structure would make it more readable

Dim printOut As String
Dim length As Long
length = Sheet1.Cells(TyLo, tnn).Value
Select Case length
    Case 0
        printOut = "Missed Call"
    Case 1 To 19
        printOut = ">1"
    Case 30 To 59
        printOut = ">30"
    Case 60 To 599
        printOut = ">60"
    Case Else
        printOut = ">600"
End Select
Sheet1.Cells(TyLo, tnn + 2).Value = printOut

I know that doesn't do exactly what yours does, but without the data I can't make many inferences.