beginner VBA question, creating macro to clean range

[ivanwakeup]

Hi guys,

very new to VBA, and i'm really interested in learning it because I find it fascinating. I know there are plenty of ways to do what i'm trying to do, but i just want to understand what it looks like in VBA so I can start picking up the language.

I want to be able to:

clean all characters I define from a given range of cells. if i could do it based on the name of the range or even the length/values within the cells in that range, that would be great. heres what I have so far, to give you an idea of what I'm trying to do:

Sub removechars()


For Each cell In Selection


ActiveCell.Value = Substitute(ActiveCell.Value, "+", "")
ActiveCell.Value = Substitute(ActiveCell.Value, "(", "")
ActiveCell.Value = Substitute(ActiveCell.Value, ")", "")
ActiveCell.Value = Substitute(ActiveCell.Value, "-", "")
Next cell


End Sub


I have no clue if this is even remotely on track or if my syntax is correct. When I try to run this, I get a compiler error: sub or function not defined. Again, what i'd like this macro to do eventually is find the proper named column within my worksheet ("Phone", in this case) and clean that range accordingly. Can someone offer any help?

Thanks very much in advance

 

[ivanwakeup]

awesome i think im making some real progress thanks guys.

so this

Sub StripPhone1()

Dim iHeaderRow        As Range
Dim rgStripCol           As Range
Dim rgHeaderCell       As Range
Dim rgStripCell          As Range


iHeaderRow = 1


For Each rgHeaderCell In Intersect(iHeaderRow, ActiveSheet.UsedRange)
   If InStr(rgHeaderCell.Value, "phone") + InStr(rgHeaderCell.Value, "Phone") > 0 Then
      Set rgStripCol = Intersect(rgHeaderCell.EntireColumn, ActiveSheet.UsedRange)
      For Each rgStripCell In rgStripCol
         If rgStripCell.Row > iHeaderRow Then
                 rgStripCell.Value = Application.WorksheetFunction.Substitute(rgStripCell.Value, "+", "")
                 rgStripCell.Value = Application.WorksheetFunction.Substitute(rgStripCell.Value, "(", "")
                rgStripCell.Value = Application.WorksheetFunction.Substitute(rgStripCell.Value, ")", "")
                 rgStripCell.Value = Application.WorksheetFunction.Substitute(rgStripCell.Value, "-", "")
                 rgStripCell.Value = Application.WorksheetFunction.Substitute(rgStripCell.Value, " ", "")
         End If
      Next rgStripCell
   End If
Next rgHeaderCell


End Sub

is what i'm looking at now. I'm trying to make it so i don't have to select a cell in my header row before running the macro, and this code produces an error for me. I'm guessing its because i don't know what variable i need to define iHeaderRow as AND i don't know how the intersect function is working


Jason - your code works perfectly, just had to add one more .replace to account for extra spaces :D. i'm still curious how i need to alter what I currently have to get dcardno's code to work as well (as i mentioned before, my goal isn't necessarily just to get a bit of code that formats my sheet properly, but rather to learn as much as possible!)

hiker95 - thanks for the resources. i've actually started watching the datapig technologies tutorials already and they are fascinating.

 

[hiker95]

ivanwakeup,

Thanks for the feedback.

You are very welcome. Glad I could help.

And, come back anytime.

 

[jasonb75]

i'm still curious how i need to alter what I currently have to get dcardno's code to work as well (as i mentioned before, my goal isn't necessarily just to get a bit of code that formats my sheet properly, but rather to learn as much as possible!)

The first error that I can see is an error in your range definition.

iHeaderRow = 1

Should be 'Set' as a range, not = to a value, a couple of examples would be

Set iHeaderRow = Rows("1:1")
Set iHeaderRow = Range("1:1")
Set iHeaderRow = Range("A1").EntireRow

The Intersect function, as it's name might suggest, looks at the point where the ranges intersect / overlap, for example, the intersection of columns E:G and rows 7:10 would be E7:G10.

 

[dcardno]

Actually, the reason for determining the "iHeaderRow" variable was that the data may come in with three or four rows of tombstone information (source, date created, client name, etc) that we want to retain, and then have the headers indicating the content of the data fields, and the header location might vary, depending on the source of the data. The user would place the cursor in the row with the headers before running the routine, and then iHeaderRow (it was dimmed as a 'long') was determined as "selection.row". After identifying the columns with 'phone' in the header you test before you strip characters out of each cell; is it in a row numerically higher (further down the sheet) than the header row? Presumably, you don't want to strip non-numeric characters out of the tombstone data, or the headers themselves. Note that testing the lower case value against 'phone' (or the uppercase value against 'PHONE')is better practice than testing against both 'phone' and 'Phone' as done in one of the other solutions - it may be (very slightly) faster, and it will catch typos in the source data - if someone mistakenly typed PHone, for example. I can only claim that I had a brain cramp when I coded my response!

I hope that helps,

 

[ivanwakeup]

thanks for the reply dcardno. I definitely agree with your logic for assigning the selected row to the variable, but in this case my header row (should) always be the first row. and yes i don't want to clean anything within the header row itself (well, i DO...but i think that involves a different project or adding some sort of code to vlookup and insert standardized header names), so i see why the "If rgStripCell.Row > iHeaderRow Then" is present.

also thanks for the clarification Jason. If anyone is curious, here's my rendition of dcardno's code to format properly. i've only tested it a bit, but it seems to be working great!

Sub StripPhone1()


Dim iHeaderRow        As Range
Dim rgStripCol           As Range
Dim rgHeaderCell       As Range
Dim rgStripCell          As Range




Set iHeaderRow = Range("a1").EntireRow




For Each rgHeaderCell In Intersect(iHeaderRow, ActiveSheet.UsedRange)
   If InStr(rgHeaderCell.Value, "phone") + InStr(rgHeaderCell.Value, "Phone") > 0 Then
      Set rgStripCol = Intersect(rgHeaderCell.EntireColumn, ActiveSheet.UsedRange)
      For Each rgStripCell In rgStripCol
         If rgStripCell.Row > iHeaderRow.Row Then
                 rgStripCell.Value = Application.WorksheetFunction.Substitute(rgStripCell.Value, "+", "")
                 rgStripCell.Value = Application.WorksheetFunction.Substitute(rgStripCell.Value, "(", "")
                rgStripCell.Value = Application.WorksheetFunction.Substitute(rgStripCell.Value, ")", "")
                 rgStripCell.Value = Application.WorksheetFunction.Substitute(rgStripCell.Value, "-", "")
                 rgStripCell.Value = Application.WorksheetFunction.Substitute(rgStripCell.Value, " ", "")
         End If
      Next rgStripCell
   End If
Next rgHeaderCell




End Sub

 

[ivanwakeup]

alright, another question guys--

why do i need to specify:

Set iHeaderRow = Range("a1").EntireRow



For Each rgHeaderCell In Intersect(iHeaderRow, ActiveSheet.UsedRange)

rather than just

Set iHeaderRow = Range("a1").EntireRow



For Each rgHeaderCell in iHeaderRow

? I don't know if this is something i'm not understanding with the logic of what my code is saying, but the second method produces an error. in my mind i don't need to specify the intersect if i've set my iheaderrow range to the entire first row?

 

[jasonb75]

What error does it give you?

The first method using Intersect restricts the code to the used range, which might only be 10 columns.

The second method would process the empty columns as well, all 16374 of them.

 

[ivanwakeup]

error 13, type mismatch and highlights:

If InStr(rgHeaderCell.Value, "phone") + InStr(rgHeaderCell.Value, "Phone") > 0 Then

 

[dcardno]

You have asked Excel to examine every cell in the first row (by not limiting the test to the intersection of the first row and the UsedRange). As it steps to the right of the used range it encounters empty cells - and the InStr() comparison fails (actually, returns a Null) - see InStr Function. The code then tries to test a Null against a value - and I think that generates a type mismatch.

 

[Teeroy]

Here's a way to do what you ask that is programatically more efficient (it does away with the looping).

Sub CleanPhoneNumbers()

Dim Rng As Range
Const sFindHeading As String = "Phone"

Application.ScreenUpdating = False
With ActiveSheet
    Set Rng = .Rows("1:4").Find(what:=sFindHeading, LookIn:=xlValues, lookat:=xlPart)
    If Rng Is Nothing Then
        MsgBox "Heading Not Found"
        Application.ScreenUpdating = True
        Exit Sub
    Else
        Set Rng = Application.Intersect(Rng.EntireColumn, .UsedRange)
    End If
    With Rng
        .Replace "(", ""
        .Replace ")", ""
        .Replace "+", ""
        .Replace "-", ""
    End With
End With
Application.ScreenUpdating = True
End Sub