817666c751d2fbebeba726ba52d1c977

Ok So i am hoping to get some feedback on this code for excel 2002 vba ?
I know is VBA but come on.... help a man out.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
Sub cleanupSheet()
' This function asks for two input the first is the sheet you want to clean up 
' The second is the Column and cell you wish to start at, e.g. A1

    Dim testRight As String
    Dim testLeft As String
    Dim stringLength As Long
    Dim endRange As String
    Dim strRange As String
    Dim inputSheetname As String
        
    inputSheetname = InputBox(Prompt:="Enter the Sheetname you want to cleanup.", _
          Title:="ENTER SHEETNAME", Default:="Group_Summary")
          
    strRange = InputBox(Prompt:="Enter the first Column/Cell.", _
          Title:="ENTER RANGE", Default:="A6")
    
    Sheets(inputSheetname).Activate
        
    ' Turn off screen updating to speed up macro.
    Application.ScreenUpdating = False

    ' Loop through the "Sheet" list and clean it up 

    endRange = left(strRange, 1)
    endRange =  endRange + CStr((Range(strRange, Range(strRange).End(xlDown)).Rows.count) + Right(strRange, 1))
    
    For Each x In Sheets(inputSheetname).Range(strRange, endRange)
                
        'See if we are looking at a field with a name in it
        testLeft = Left(x.Value, 1)
        If testLeft Like "[a-z, A-Z]" Then
            
            'Test for numbers
            testRight = Right(x.Value, 1)
            
            Do While testRight Like "[0-9]"
               
               stringLength = Len(x.Value)
               stringLength = stringLength - 1
               
               'Remove right most charater from cell
               x.Value = Left(x.Value, stringLength)
               testRight = Right(x.Value, 1)
            Loop
                        
            'Test for know String Values
            testRight = Right(x.Value, 3)
            If testRight = " OC" Then
                'Remove value from right side of cell
                stringLength = Len(x.Value)
                stringLength = stringLength - 2
                x.Value = Left(x.Value, stringLength)
            
            ElseIf testRight = " NH" Then
            
            'Remove value from right side of cell
                stringLength = Len(x.Value)
                stringLength = stringLength - 2
                x.Value = Left(x.Value, stringLength)
            End If
                     
        End If
        'Trim spaces from both sides of cell contents
        x.Value = Trim(x.Value)
    Next x
    
  Application.ScreenUpdating = True
MsgBox "Done!"
End Sub

Refactorings

No refactoring yet !

0481b9ea2b2aad6844b33dbca2d7fe0e

pkemper

December 18, 2007, December 18, 2007 13:26, permalink

1 rating. Login to rate!

You do a lot of assignments in between. That takes unnecessary cycles:

stringLength = Len(x.Value)
stringLength = stringLength - 1
'Remove right most charater from cell
x.Value = Left(x.Value, stringLength)

Is probably quicker this way:
x.Value = Left( x.Value, Len(x.Value)-1 )

If I understand correctly, cleanup means to keep all strings that start with letters a-z,A-Z and remove any ' NHnnnn' and ' OCnnnn'

Maybe this test for numbers at the end of a string is quicker:
do
r = Right(x.Value,1)
again = 0
If ( r >= '0' And r <= '9' )
x.Value = Left( x.Value, Len( x.Value ) - 1 )
again = 1
EndIf
while ( again = 1 )

Your refactoring





Format Copy from initial code

or Cancel