Check my code please! Match & Delete

[Simon Lloyd]

I have written some code to help another person, the criteria was to compare Range_A against Range_B and delete any matches in Range_A that are found in Range_B

My questions are (as i havent posted this solution yet) is this a safe way to do it, am i missing something?
Sub DelDuplicate()
Dim Rng As Range, Rng2 As Range, Str
Dim MyCell

Str = MyCell

Set Rng = Range("Range_A")
Set Rng2 = Range("Range_B")

Application.ScreenUpdating = False

For Each Str In Rng
If Not IsError(Application.Match(Str, Rng2, 0)) Then
Str.Delete
End If
Next

Application.ScreenUpdating = True
End Sub
Regards,
Simon

[davemcochran]

Does it work??

[Simon Lloyd]

Yes it works fine, just need a professional opinion on whether i can do it smarter or whether im missing something!

Regards,
Simon

[davemcochran]

This works very well,
Are your named ranges Dynamic??

[Simon Lloyd]

Dave thanks for the replies, no the named ranges aren't dynamic. If you dont mind me asking are you experienced with excel, if so any comments or criticisms with my code?

Reagrds,
Simon

[Norie]

Simon

There is one problem with the code - the use of Str as a variable name.

Str is a VBA function so you shouldn't use it.

Also what exactly will you being deleted and how should cells move after the deletion? Up/down, left/right?

[Simon Lloyd]

Thanks Norie i can change that, when the matched cell is found and deleted all cells move up but that seemed to happen automatically, how would i have moved the cells other wise lets say left for arguments sake. After Dave prompting me about dynamic named ranges i wrote this =OFFSET(Range_A!$A$1,0,0,COUNTA(Range_A!$A:$A),1)but now the code will not work i get runtime error 1004 application not defined error, is there something different i need to do to reference a dynamic named range?

Regards,
Simon

[mdmackillop]

Hi Simon,
A few points.
1. Most important, the delete method does not work well in this type of loop. If you have repeating values in Range_A, they will not all be deleted. see attachment
2. Include the Option Explicit statement
3. Do capitals matter in your comparison? You may want to add Option Compare Text.
4. Str should also be declared as string
5. MyCell is not used or neccessary and can be deleted.
6. For good practice, the declared ranged should be set to = 0 on completion.
7. Best to state the ShiftDirection when using Delete, otherwise Excel decides it for you.
8. Of lesser importance, if you're only using a range once, there is no real benefit in setting it to a range variable

To delete in a loop, start with the last cell and work up.
eg for x = LastRow to 1 Step - 1

OK. Maybe more than a few points, but you have the general idea. Tidying up just takes a little practice.
Regards
MD

Edit. I missed Norie's point re Str, but it's a good one.

[Simon Lloyd]

Malcom thanks, could you check your PM's please!

Regards,
Simon

[mdmackillop]

Hi Simon
The Dynamic range would be
=OFFSET(Sheet1!$A$1,0,0,COUNTA(Sheet1!$A:$A),1)
There is a difficulty here though, if A1 is deleted by your code, $A$1 becomes #Ref and the range name fails.
You can reinstate it with further code

'Reset range name
ActiveWorkbook.Names.Add Name:="range_a", RefersToR1C1:= _
"=OFFSET(Sheet1!R1C1,0,0,COUNTA(Sheet1!C1),1)"

[Bob Phillips]

I have written some code to help another person, the criteria was to compare Range_A against Range_B and delete any matches in Range_A that are found in Range_B

My questions are (as i havent posted this solution yet) is this a safe way to do it, am i missing something?
Sub DelDuplicate()
Dim Rng As Range, Rng2 As Range, Str
Dim MyCell

Str = MyCell

Set Rng = Range("Range_A")
Set Rng2 = Range("Range_B")

Application.ScreenUpdating = False

For Each Str In Rng
If Not IsError(Application.Match(Str, Rng2, 0)) Then
Str.Delete
End If
Next

Application.ScreenUpdating = True
End Sub
Regards,
Simon

- declare all variable types (Str, MyCell is undeclared). If you mean them to be variant, say so

- you set Str = MyCell as the frirst uinstructions, what is MyCell, where does it come from ( I hope it isn't a global variable!)? Also it is pointless as Str is the loop index, so it gets over-written immediatlely

- add an error cat6ch (unless you already have it in the caller)

- turn screen updating off as soon as is possible

- is your delete doing a whole row, it is not clear? If so, you can miss rows going top-down, better to catch them as you go, and delete en-masse at the end

- where is the function synopsis (comments)

- as someone else said, Str is not a good name, even if it works

[Simon Lloyd]

Thanks for the reply Malcom,

The name of the range is Range_A was i wrong in changing Sheet1! for Range_A! so where i set the Rng = Range("Range_A") would i place the =offset statement in these brackets?, if not how do i reference a dynamic named range?

Regards,
Simon

[Simon Lloyd]

Sorry Bob you must have posted as i did,
is your delete doing a whole row, it is not clear?i am just deleting one cell at a time
turn screen updating off as soon as is possibleShould i have turned off updating right after the DIM's? i thought where i had turned it off was before the task was being performed.
declare all variable types (Str, MyCell is undeclared). If you mean them to be variant, say soprobably my laziness as i thought if you do not DIM As then they are automatically a variant....but i understand for legibility and averting confusion i should have.
add an error cat6ch (unless you already have it in the caller) should i have created something like
On Error Goto Xit
CODE
Xit:
MsgBox" An Error Occurred Function Not Performed"
Exit Sub


you set Str = MyCell as the frirst uinstructions, what is MyCell, where does it come from ( I hope it isn't a global variable!)? Also it is pointless as Str is the loop index, so it gets over-written immediatlelyI didnt even notice that there was a problem there...just amature coding!
where is the function synopsis (comments)
did you mean in my code Bob? i.e what each portion is doing?

Thanks for the pointers and info, it's the only way i can learn by attempting something post it and have the experts take a look.

Regards,
Simon

[mdmackillop]

The Offset formula is entered into the Insert/Names dialog. This can be done manually or by code. I'm not sure if it is the best way in this example. I would probably forget Range_A & Range _B and go straight to Rng, Rng2 as follows

set Rng=Range(Cells(1,1), Cells(Rows.Count,1).End(xlup))