0

My knowledge on VBA is limited, so I struggle even with this simple task. I want to check every cell in column B, if that cell value is duplicated in an another worksheet's column A. Then if it has duplicate values in that range, it colors the cell to yellow. Ive got this part working. The next thing I want to do is to color the cells in that same row in columns C, D and F. This part is not working for me, it gives me an Type Mismatch error. Any help on this would be appreciated.

Sub Mark_Duplicates()

    Dim Cell As Variant
    Dim Source As Range
    Dim Source2 As Range
    Dim rownumber As Variant

    Set Source = Range("B1:B1000")
    Set Source2 = Worksheets("Chain").Range("A1:A1000")

    For Each Cell In Source

        If Application.WorksheetFunction.CountIf(Source2, Cell) > 1 Then

            rownumber = ActiveCell.Row
            Cell.Interior.Color = RGB(255, 255, 0)
            Range("C" & rownumber).Interior.Color = RGB(255, 255, 0)
            Range("D" & rownumber).Interior.Color = RGB(255, 255, 0)
            Range("F" & rownumber).Interior.Color = RGB(255, 255, 0)

        End If

    Next

End Sub
Error 1004
  • 7,877
  • 3
  • 23
  • 46

1 Answers1

1

As per @BigBen, the most obvious thing to correct in your code is the way you concatenate a range. You have currently used the arithmetic addition (+) whereas the correct VBA syntax would be the &.

With that out of the way, you also shouldn't refer to the ActiveCell. Since you never activate any other cell, the referenced row will always stay the same. In general, as per my comment, to refer to something active is bad practice. Instead use Cell.Row.

Another good practice would be to use Long data type variables for full integer numbers instead of data type Variant. Amongst the disadvantages to leave it up to VBA to decide on a variant data type, is that using Long takes 4 bytes, whereas a variant using numbers takes 22. So, you are slowing down your code. In fact, you won't need it per se.

You can also, instead of coloring cell by cell, color a range at once. You can simply refer to a larger range.

A last note is that you never fully qualified, at the very least, the referenced sheet for Source, which in that case will refer to B1:B1000 on again, the ActiveSheet. I personally prefer to refer to a sheets CodeName.

To conclude, you could re-write your code like:

Sub Mark_Duplicates()

Dim Source1 As Range: Set Source1 = Sheet1.Range("B1:B1000") 'Use a worksheet's CodeName instead of sheet name (which can change)
Dim Source2 As Range: Set Source2 = Sheet2.Range("A1:A1000")
Dim cell As Range

For Each cell In Source1
    If Application.CountIf(Source2, cell) > 1 Then
        Sheet1.Range(Replace("B?:D?,F?", "?", cell.Row)).Interior.Color = RGB(255, 255, 0)
    End If
Next

End Sub

If your range would grow to a larger dataset, it's wise to no longer iterate over a Range but instead switch to an array which makes for much faster processing.

Question remains, do you even need VBA at all (as per @BigBen), as you could do this through conditional formatting (the larger the range, the slower your project as CF is volatile per definition)

JvdV
  • 70,606
  • 8
  • 39
  • 70
  • Thank you, this was very informative for me. I will use this VBA code to mark some important information for me, print it, then delete the data. This data changes multiple times a day, so I think using a VBA is better over the Conditionla formatting, as for this I just need to click a button. – Tomas Perlecky Oct 28 '19 at 19:48
  • @TomasPerlecky, in that case you'll definitely be interested in findin a last used row and using arrays instead of range objects. – JvdV Oct 29 '19 at 05:45