0

I've written a loop that runs through a range containing month names and to trim any that are greater than three characters as I only need to see the first three ie: Jan instead of January.

The code below works in identifying the cells that contain the longer names but the LEFT function clears the cell rather than just removing the excess characters to show the first three only. Any idea what is amiss in the function? Help is much appreciated.

Many thanks.

Sub TrimMonth()
    Application.ScreenUpdating = "False"

    Dim rng As Range
    Dim i, counter As Integer
    Dim lastrow As Long

    lastrow = ActiveSheet.Range("A1048576").End(xlUp).row

    'Set the range to evaluate.
    Set rng = Range("A2:A" & lastrow)

    'initialize i to 1
    i = 1

    'Loop for a count of 1 to the number of rows in
    'the range to evaluate.
    For counter = 1 To rng.Rows.Count
        'If cell i in the range contains more than 3
        'characters then trim to 3 characters else increment i
        If Len(rng.Cells(i)) > 3 Then
            rng.Cells(i).Value = Left(Cells(i).Value, 3)
            i = i + 1
        Else
            i = i + 1
        End If
    Next

    Application.ScreenUpdating = "True"   
End Sub
Pᴇʜ
  • 56,719
  • 10
  • 49
  • 73
Marc
  • 45
  • 4
  • 4
    You forgot the `rng` qualifier: `rng.Cells(i).Value = Left(rng.Cells(i).Value, 3)` – Rory Dec 11 '18 at 10:18
  • 1
    Note that `Dim i, counter As Integer` only declares `counter As Integer` but `i As Variant` you must specify a type for **every** variable. Also you must use `Long` for row counting variables because Excel has more rows than `Integer` can handle: `Dim i As Long, counter As Long` • Regardless I recommend [always to use Long instead of Integer](https://stackoverflow.com/a/26409520/3219613) in VBA since there is no benefit in `Integer` at all. – Pᴇʜ Dec 11 '18 at 10:42
  • Variable i is superfluous, you can omit i and the i=i+1 and directly use your counter variable instead. Also, using a construction like For Each TheCell in rng is faster than using a row counter. – jkpieterse Dec 11 '18 at 10:44
  • The problem is that `i = 1` and your range starts at row 2. – TinMan Dec 11 '18 at 11:38
  • You should watch: [Excel VBA Introduction Part 5 - Selecting Cells (Range, Cells, Activecell, End, Offset)](https://www.youtube.com//watch?v=c8reU-H1PKQ&index=5&list=PLNIs-AWhQzckr8Dgmgb3akx_gFMnpxTN5) [Excel VBA Introduction Part 16 - For Next Loops](https://www.youtube.com//watch?v=JyWrLH7monI&index=18&list=PLNIs-AWhQzckr8Dgmgb3akx_gFMnpxTN5) [Excel VBA Introduction Part 17 - For Each Loops](https://www.youtube.com//watch?v=R2nlDu-2E4o&index=19&list=PLNIs-AWhQzckr8Dgmgb3akx_gFMnpxTN5) – TinMan Dec 11 '18 at 11:41
  • Why not just format the cells? – Solar Mike Dec 11 '18 at 11:59
  • @TinMan Note that there are 2 different countings. While the `i` in `Range.Cells(i)` is *relative* to the `Range`, the `i` in `Worksheet.Cells(i)` is *absolute* to the worksheet. In this case `rng.Cells(1)` is the same cell as `ActiveSheet.Columns("A").Cells(2)` because the first cell in `rng` is in the worksheet row 2. • Other Example: `Range("A100:A200").Row(5)` is actually `Row(105)` in the worksheet. – Pᴇʜ Dec 11 '18 at 14:11
  • @Pᴇʜ your are correct. I didnt notice the second counting, The OP is using `Cells(i).Value`. So iif `i=5` then then `Cells(i).Value` would refer to `E1`. You should post an answer. – TinMan Dec 11 '18 at 19:51

1 Answers1

1

This code adds a formula to column B to return the three letter month text, then copies the values to column A before deleting the formula.

Sub TrimMonth()

    Dim rDates As Range

    With ThisWorkbook.Worksheets("Sheet1")

        'Set reference to range containing month names.
        Set rDates = .Range(.Cells(2, 1), .Cells(.Rows.Count, 1).End(xlUp))

        'Add formula one column to right.
        'This will convert the month name to a real date and then format it
        'as three letter month text.
        rDates.Offset(, 1).FormulaR1C1 = _
            "=TEXT(DATEVALUE(""1-"" & RC[-1]),""mmm"")"

        'Replace originals with values from formula.
        rDates.Value = rDates.Offset(, 1).Value

        'Clear formula.
        rDates.Offset(, 1).ClearContents

    End With

End Sub  

Or to do it without adding the formula:

Sub TrimMonth()

    Dim rDates As Range
    Dim rCell As Range

    With ThisWorkbook.Worksheets("Sheet1")

        'Set reference to range containing month names.
        Set rDates = .Range(.Cells(2, 1), .Cells(.Rows.Count, 1).End(xlUp))

        'Convert each cell in range.
        For Each rCell In rDates
            rCell.Value = Format(CDate("1-" & rCell), "mmm")
        Next rCell

    End With

End Sub
Darren Bartrup-Cook
  • 18,362
  • 1
  • 23
  • 45