1

I'm trying to delete the last comma from each cell of a dynamic range.

The macro doesn't delete the comma, it just selects the range.

Sub selecting()
Dim sht As Worksheet
Dim LastColumn As Long
Dim StartCell As Range

Set sht = Worksheets("Sheet1")
Set StartCell = Range("D1")

'Find Last Row and Column
LastRow = sht.Cells(sht.Rows.Count, StartCell.Column).End(xlUp).row
LastColumn = sht.Cells(StartCell.row, sht.Columns.Count).End(xlToLeft).Column

'Select Range
sht.Range(StartCell, sht.Cells(LastRow, LastColumn)).Select

With ActiveCell
    If Right(.Value, 1) = "," Then .Value = Right(.Value, Len(.Value) - 1)

End With

End Sub

Here is what is returned

enter image description here

Community
  • 1
  • 1
AleksLi
  • 27
  • 5
  • 2
    `.Value = Left(.Value, Len(.Value) - 1)` – braX Dec 30 '19 at 23:23
  • 1
    I'm pretty sure you need to loop through the cells in the selected range...? – Danny Papadopulos Dec 30 '19 at 23:44
  • 1
    "Be careful to distinguish between the active cell and the selection. The active cell is a single cell inside the current selection. The selection may contain more than one cell, but only one is the active cell." - [MSDN](https://learn.microsoft.com/en-gb/office/vba/api/excel.window.activecell) – barrowc Dec 30 '19 at 23:44
  • Try to [avoid using `Select`](https://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba) – Chronocidal Dec 31 '19 at 00:35
  • You can do this simply in Power Query with `Trim.End`, but maybe a better approach would be to change the method you used to construct the strings in the first place. It might be simpler to change that code. – Ron Rosenfeld Dec 31 '19 at 12:15

3 Answers3

1

Since you want to remove the last character from each cell in column D, try this variation on braX's comment. It loops trough each used cell in column 4 and deletes the last character.

With ThisWorkbook.Sheets("Sheet1")
    For Each cel In .Range("D1", .Cells(.Rows.Count, 4).End(xlUp))
        cel.Value = Left(cel, Len(cel) - 1)
    Next cel
End With
chris neilsen
  • 52,446
  • 10
  • 84
  • 123
GMalc
  • 2,608
  • 1
  • 9
  • 16
  • 1
    I think looping is the correct way of doing this, however OP wants a check for a comma. Otherwise you are removing the last character unwanted. Also, you are using an implicit sheet reference on `Cells(Rows.Count...)` – JvdV Dec 31 '19 at 08:33
  • Thanks for taking the time to look into this. Your code worked, and did exactly what I needed! – AleksLi Jan 01 '20 at 17:18
  • @AleksLi I think the problem was the code or formula you used to combine the cells added the last `,` which could probably be fixed without using this additional code. – GMalc Jan 01 '20 at 23:27
1

The most conventional way would be to loop over your cells:


Sub Replacing()

Dim lr As Long, lc As Long
Dim rng As Range, cl As Range

With Worksheets("Sheet1")

    'Find Last Row and Column
    lr = .Cells(.Rows.Count, 1).End(xlUp).Row
    lc = .Cells(1, .Columns.Count).End(xlToLeft).Column

    'Go through range
    Set rng = .Range(.Cells(1, lc), .Cells(lr, lc))
    For Each cl In rng
        If Right(cl.Value, 1) = "," Then cl.Value = Left(cl.Value, Len(cl.Value) - 1)
    Next cl

End With

End Sub

Better would be to go through memory if your range is actually much larger (for performance sake)

Sub Replacing()

Dim lr As Long, lc As Long, x As Long
Dim arr As Variant

With Worksheets("Sheet1")

    'Find Last Row and Column
    lr = .Cells(.Rows.Count, 1).End(xlUp).Row
    lc = .Cells(1, .Columns.Count).End(xlToLeft).Column

    'Go through array
    arr = .Range(.Cells(1, lc), .Cells(lr, lc)).Value
    For x = LBound(arr) To UBound(arr)
        If Right(arr(x, 1), 1) = "," Then arr(x, 1) = Left(arr(x, 1), Len(arr(x, 1)) - 1)
    Next x

    'Write array back to range
    .Range(.Cells(1, lc), .Cells(lr, lc)).Value = arr

End With

End Sub

And a more less conventional way (alright for small ranges I guess) would be to evalate a range and avoid an iteration. This however comes at the cost of an array formula:

Sub Replacing()

Dim lr As Long, lc As Long
Dim rng As Range

With Worksheets("Sheet1")

    'Find Last Row and Column
    lr = .Cells(.Rows.Count, 1).End(xlUp).Row
    lc = .Cells(1, .Columns.Count).End(xlToLeft).Column

    'Evaluate your range
    Set rng = .Range(.Cells(1, lc), .Cells(lr, lc))
    rng.Value = .Evaluate("IF(RIGHT(" & rng.Address & ",1)="","",LEFT(" & rng.Address & ",LEN(" & rng.Address & ")-1)," & rng.Address & ")")

End With

End Sub
JvdV
  • 70,606
  • 8
  • 39
  • 70
-1

Use the Replace() function:

For Each Cell in Range.Cells
    Cell.Value = Replace(Cell.Text, ",", "")
Next Cell

Edit: After testing, replaced .Text with .Value

Edit 2: I'd also like to add, why are you selecting the range? My supposition is that you are selecting it to enable use of ActiveCell but you can simply manipulate the range without selecting it. Selecting the range is asking to incur errors. My suggestion is:

Dim rplcRng as Range
Set rplcRange = sht.Range(StartCell, sht.Cells(LastRow, LastColumn))

For Each Cell in rplcRng.Cells
    Cell.Value = Replace(Cell.Text, ",", "")
Next Cell

Edit 3: added "s"s

jclasley
  • 668
  • 5
  • 14
  • 1
    Down voting because your code replaces all commas in the cells, not the last comma, as the OP ask. The correct approach is the one in the comment by @braX. – GMalc Dec 31 '19 at 03:45