CODEWITHSUNDEEP
vbaexcel
user2156096
user2156096

Reputation: 43

How to change my code to get the values into arrays

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!

Upvotes: 0

Views: 85

Answers (2)

NickSlash
NickSlash

Reputation: 5077

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)

Upvotes: 1

sehe
sehe

Reputation: 392989

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!

Upvotes: 0

Related Questions