VBA code in excel operates inconsistently with very simple code

Question

I wrote some pretty simple VBA (excel macros) code to manage my audio licencing excel experience. The code is supposed to look through the excel sheet in column 3, look for any that have "AMC" in their column, and then copy and paste the row to sheet 2 and continue searching through entire excel document. This code is very simple and worked once right before it stopped working right. It only takes the very last AMC value and puts that on sheet 2 but not the other 5 rows that have AMC in their column 3 value.

Please help! I would appreciate it very much :)

-Jeremy

VBA Code:

Sub CommandButton1_Click()

    a = Worksheets("Sheet1").UsedRange.Rows.Count

    b = 0

    For i = 2 To a

        If Worksheets("Sheet1").Cells(i, 3).Value = "AMC" Then

            Worksheets("Sheet1").Rows(i).Copy

            Worksheets("Sheet2").Activate

           ' b = ActiveSheet.UsedRange.Rows.Count

            Worksheets("Sheet2").Cells(b + 1, 1).Select

            ActiveSheet.Paste

            Worksheets("Sheet1").Activate

        End If

    Next

    Application.CutCopyMode = False

    ThisWorkbook.Worksheets("Sheet1").Cells(1, 1).Select

End Sub

Continuing search through the entire document or just column C sheet 3? — QHarr
– QHarr, Commented Jun 27, 2018 at 5:35
It seems that you are not incrementing your b variable. You always select b+1 and b is always 0. — eren
– eren, Commented Jun 27, 2018 at 5:39

eren · Accepted Answer · 2018-06-27 05:43:59Z

1

This should solve your problem :

  If Worksheets("Sheet1").Cells(i, 3).Value = "AMC" Then    
        Worksheets("Sheet1").Rows(i).Copy  
        Worksheets("Sheet2").Activate   
        Worksheets("Sheet2").Cells(b + 1, 1).Select    
        b = b + 1    
        ActiveSheet.Paste   
        Worksheets("Sheet1").Activate        
  End If

answered Jun 27, 2018 at 5:43

eren

81010 silver badges23 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

Jeremy F Over a year ago

Oh my god... I cant believe I wasn't kicking the variable b to the next value.... Thank you SO MUCH! Seriously. I feel kind of like an idiot now haha.

eren Over a year ago

You are welcome, glad i could help. But to be honest take a look at answer from @QHarr. Doing things more efficiently should be your goal :)

eren Over a year ago

@JeremyF Btw you can mark the answer if it solved your problem ;) meta.stackexchange.com/questions/147531/…

QHarr · Accepted Answer · 2018-06-27 06:04:50Z

1

You could use Instr and Union.

Union is very efficient as you store all the qualifying ranges in memory and then write out only once to the sheet. Much less expensive operation than continually writing out to the sheet.
Instr allows you to use vbBinaryCompare which means you are doing a case sensitive match i.e. only AC not ac will be matched on.
The code belows avoids .Activate, which is again an expensive operation that isn't required.
UsedRange means you may be looping many more rows than required. You only want to loop to the last populated row in column C of sheet 1, as that is the column you are testing. Hence, I use .Cells(.Rows.Count, C").End(xlUp).Row to find that last row.
Use Option Explicit - research why! It will make your VBA life soooooo much better.

Code:

Option Explicit    
Sub CommandButton1_Click()

    Dim lastRow As Long, sSht As Worksheet, tSht As Worksheet, loopRange As Range
    Set sSht = ThisWorkbook.Worksheets("Sheet1")
    Set tSht = ThisWorkbook.Worksheets("Sheet2")

    With sSht
        Set loopRange = .Range("C2:C" & .Cells(.Rows.Count, C").End(xlUp).Row)
    End With

    Dim rng As Range, unionRng As Range
    For Each rng In loopRange
        If InStr(1, rng.Value, "AC", vbBinaryCompare) > 0 Then
            If Not unionRng Is Nothing Then
                Set unionRng = Union(unionRng, rng)
            Else
                Set unionRng = rng
            End If
        End If
    Next rng
    If Not unionRng Is Nothing Then unionRng.EntireRow.Copy tSht.Cells(1, 1)    
End Sub

edited Jun 27, 2018 at 6:04

answered Jun 27, 2018 at 5:45

QHarr

84.5k14 gold badges58 silver badges105 bronze badges

2 Comments

Jeremy F Over a year ago

Thank you so much for your input but the former person had already figured it out. Thank you so much for trying :) Seriously, I appreciate you both very much.

QHarr Over a year ago

I'm pleased. I would give some serious thought to the points I raise above if you want to write more efficient code.

Collectives™ on Stack Overflow

VBA code in excel operates inconsistently with very simple code

2 Answers 2

3 Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

3 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related