Macro not moving the string after the character found

Question

I am trying to move the cell value to adjacent cell if the string contains @ character in it. but the following Macro is not working as intended.

Sub Macro1()
  Dim MatchString As String
  MatchString = "@"
  For Counter = 1 To Range("A:A").Count
    If (InStr(Range("A" & Counter).Value, Len(MatchString)) = MatchString) Then
      Range("A" & Counter).Select
      Selection.Cut
      Range("B" & Counter).Select
      ActiveSheet.Paste
    End If
  Next Counter
End Sub

Kindly suggest me what I missed in this macro so that my program works well.

You are also only looking if the left character is `@`, not if the string contains `@` — Scott Craner, May 26 '17 at 13:52
`If "myCell.Value" Like "*@*" Then` can also be helpful. Look [here](https://learn.microsoft.com/en-us/dotnet/articles/visual-basic/language-reference/operators/like-operator) — M--, May 26 '17 at 13:55
@Masoud Can you please help me where I need to edit in the code? — Jaffer Wilson, May 26 '17 at 13:56
Now you are trying to find `1` in the string with `InStr(Range("A" & Counter).Value, Len(MatchString))` remove the `Len()` as it returns a number of the length of the string. `InStr(MatchString,Range("A" & Counter).Value)`. please research what each part does and how to use it, we are not wikipedia. — Scott Craner, May 26 '17 at 13:59
You know `Range("A:A").Count` will take forever to run? Apply this as well [How to loop through a column](https://stackoverflow.com/questions/27000953/excel-vba-loop-through-cells-in-a-column-if-not-empty-print-cell-value-into) — M--, May 26 '17 at 14:06
Have timed the two answers with 160 entries of `@` - FIND method took 47 ms, whole column took 10093ms. — Darren Bartrup-Cook, May 26 '17 at 14:16

M-- · Accepted Answer · 2017-05-26T14:13:52.483

2

This should work; using Like

Sub Macro1()

  With ActiveSheet
  For Counter = 1 To .Range("A:A").Count
    If .Range("A" & Counter).Value Like "*@*" Then

      .Range("A" & Counter).Cut .Range("B" & Counter)
      Application.CutCopyMode = False

    End If
  Next Counter
  End With

End Sub

You should avoid selecting/activating in VBA:

How to avoid using Select in Excel VBA macros

If you don't really want to loop over an entire column read this:

EXCEL VBA - Loop through cells in a column, if not empty, print cell value into another column

As posted as another answer, Find would be a good approach (faster) toward this problem.

edited May 26 '17 at 14:13

answered May 26 '17 at 14:00

M--

25,431
8
61
93

Thank you. it really helped. – Jaffer Wilson May 26 '17 at 14:06
1

++ perfect answer. Just to add, after cut/copy its good to add Application.CutCopyMode=False. No harm. :) – cyboashu May 26 '17 at 14:10

score 2 · Answer 2 · answered May 26 '17 at 14:08

Rather then looking at every cell in column A just go straight to the cells with FIND.

Each time @ is found it will be moved to column B and the value in column A is removed. When there's no more to find the loop will stop.

Public Sub MoveToAdjactent()

    Dim MatchString As String
    Dim rFound As Range

    MatchString = "@"

    With ThisWorkbook.Worksheets("Sheet1").Columns(1)
        Set rFound = .Find(MatchString, LookIn:=xlValues, LookAt:=xlPart)
        If Not rFound Is Nothing Then
            Do
                rFound.Offset(, 1) = rFound
                rFound.ClearContents
                Set rFound = .FindNext(rFound)
            Loop While Not rFound Is Nothing
        End If
    End With

End Sub

That's how you should do it. Loops are bad practices. +1 – M-- May 26 '17 at 14:10 — M--, May 26 '17 at 14:10

Macro not moving the string after the character found

2 Answers2