Return to Answer

###Manually assigning column letters

###Variables

###Error Handling

###Select

###Last Row

###Readability

Manually assigning column letters

Variables

Error Handling

Select

Last Row

Readability

Manually assigning column letters

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.

###Manually assigning column letters

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.