0

I followed some array tutorials, but my code in VBA is too difficult for me to change it into arrays for my basic knowledge.

Can anyone help?

This is my code:

Sub InternExtern()

Dim source, addrescell, destination As Range
Dim Sourcevalue As String


For Each source In Range("E6", Range("E" & Rows.Count).End(xlUp))
 If source.Value <> "" Then
    For Each addrescell In Range("address_table_names").Rows
             If addrescell.Cells(1).Value <> "" And InStr(source.Offset(0, 23).Value, "Extern") = 0 Then
               SourceName = addrescell.Cells(1).Value
               Sourcevalue = addrescell.Cells(1, 4).Value
                    If InStr(UCase(source), UCase(SourceName)) <> 0 Then
                      If InStr(Sourcevalue, "10.") <> 0 Or InStr(Sourcevalue, "192.168.") <> 0 Or IsInternal(addrescell.Offset(0, 3).Value) Then
                       source.Offset(0, 23) = "Intern"
                       Else: source.Offset(0, 23) = "Extern"
                      End If
                    End If
                      If InStr(source, "-ext-") <> 0 Or InStr(source, "any") <> 0 Or InStr(source, "-EXT-") <> 0 Then
                      source.Offset(0, 23) = "Extern"
                      End If
                      If InStr(source, "any") <> 0 And InStr(source.Offset(0, -1).Value, "FW-Peering") = 0 Then
                      source.Offset(0, 23) = "Intern"
                      End If

            End If
            Next addrescell
End If
Next source

My goal of adding the column values into arrays is to make it faster.

Thanks in advance!

Olle Sjögren
  • 5,315
  • 3
  • 31
  • 51
user2156096
  • 43
  • 1
  • 3
  • 10
  • 1
    To make _what_ faster, anyways? – sehe Mar 15 '13 at 12:37
  • You did not properly declare all your Range variables. `source` and `addrescell` are Variant based on the way you dimensioned them. – David Zemens Mar 15 '13 at 13:45
  • Your [previous question](http://codereview.stackexchange.com/q/23923/2726) got migrated to CR, and was well answered by [Dick Kusleika](http://codereview.stackexchange.com/a/23956/2726). Voting to close. – chris neilsen Mar 15 '13 at 18:38

2 Answers2

1

I don't quite see how putting something in an array would be helpful. You'd have to take it out again sometime.

You might try disabling Application.ScreenUpdating and Application.Calculation while your loop runs and turning it back on at the end.

Application.ScreenUpdating = False
Application.Calculation = xlManual
' your loop
Application.ScreenUpdating = True
Application.Calculation = xlAutomatic

If you could explain what the code is doing or what you want to do we might be able to help more. Using a worksheet function like @sehe suggested might be a much better solution.

Update

Just looked at your link to codereview and one of the answers show that its fairly simple and quick to create an array copy of your worksheet and to re-apply the modified array to the worksheet. Not seen that before, pretty cool stuff.

While Arrays might be quicker, I think in your case they would just add a lot of extra complexity as you use methods of the objects that wont be there when you convert it to an array.

Assuming you add the cell Address to the array (otherwise you have no context to the sheet)

Array(1).Offset(0,1) = <not going to happen>

Range(Array(1)).Offset(0,1) = <going to work>

So you'd be going back to objects to do your stuff, unless you have lots of arrays, that may or may be redundant.

Since I'm not 100% sure what the goal of your function is this might not help at all! I think you might have some extra (unneeded) iterations of your internal for loop. If at the sections marked 'HERE basically mean that you've completed that iteration you can exit the loop and goto the next parent loop.

For Each source In Range("E6", Range("E" & Rows.Count).End(xlUp))
    If source.Value <> "" Then
        For Each addrescell In Range("address_table_names").Rows
            If addrescell.Cells(1).Value <> "" Then ' vba doesn't shortcircuit
                If InStr(source.Offset(0, 23).Value, "Extern") = 0 Then
                    SourceName = addrescell.Cells(1).Value
                    Sourcevalue = addrescell.Cells(1, 4).Value
                    If InStr(UCase(source), UCase(SourceName)) <> 0 Then
                        If InStr(Sourcevalue, "10.") <> 0 Or InStr(Sourcevalue, "192.168.") <> 0 Or IsInternal(addrescell.Offset(0, 3).Value) Then
                            source.Offset(0, 23) = "Intern"
                            ' HERE
                            Exit For
                        Else
                            source.Offset(0, 23) = "Extern"
                            ' HERE
                            Exit For
                        End If
                    End If
                    If InStr(source, "-ext-") <> 0 Or InStr(source, "any") <> 0 Or InStr(source, "-EXT-") <> 0 Then
                        source.Offset(0, 23) = "Extern"
                        ' HERE
                        Exit For
                    End If
                    If InStr(source, "any") <> 0 Then ' again, no shortcircuit
                        If InStr(source.Offset(0, -1).Value, "FW-Peering") = 0 Then
                            source.Offset(0, 23) = "Intern"
                            ' HERE
                            Exit For
                        End If
                    End If
                End If
            End If
        Next addrescell
    End If
Next source

Information about shortcircuit-ing, Having a nested if is faster than using and, not lightspeed, but saves you one comparison. If you're doing lots of iteration it can add up (a little anyway)

Community
  • 1
  • 1
NickSlash
  • 4,758
  • 3
  • 21
  • 38
  • hey, thanks for your answer but on my previous question, they told me to make arrays. What u just said, thats what i already did. here is my previous question: http://codereview.stackexchange.com/questions/23923/improve-vba-code-excel – user2156096 Mar 15 '13 at 14:02
0

I'd recommend making a worksheet function out of it:

That way, you can just type a formula in the calculated columns

 =InternExtern(E6)

And the cell will only be recalculated if you do a full recalc, you recompile the VBA project or the source value changes!

sehe
  • 374,641
  • 47
  • 450
  • 633