Simple code.. What's wrong?

Ok.. What I'm trying to do is:

1. open a log file - CHECK
2. grab the first 15 characters of each line (an IP address) - CHECK
3. bounce these first 15 off of my array to see if they are in it, if not, then record them in the array (arrIPs)
4. Print the new IP.. only if it is one i haven't encountered thus far.


The problem:
It records the first IP and prints it out just fine, but then the second one prints in the text box multiple times.. as does the 3rd, 4th, etc..

What am i doing wrong?? Any help at all is greatly appreciated. I'm a noob! But I don't wanna be! :)

VB Code:

Private Sub cmdLogalyze_Click()
Dim arrIPs(1000) As Variant
strwrap = Chr$(13) + Chr$(10)
Chr$ (10)
txtSimple = ""
cd1.InitDir = "C:\Program Files\Apache Group\Apache2\logs"
cd1.Filter = "Log Files (*.log)|*.log"
cd1.ShowOpen
If cd1.FileName = "" Then
msg = MsgBox("No File Chosen.", vbInformation, "File Needed")
Exit Sub
End If
 
 
Open cd1.FileName For Input As #1
Do Until EOF(1)
Line Input #1, strNewLine
' get the ip string from the start of the log
strIP = (Mid(strNewLine, 1, 15))
 
'loop through the array to see if the ip is in it
For i = 1 To UBound(arrIPs)
' if it is, then go to the next line
If strIP = arrIPs(i) Then
GoTo skipwrite
' if not, then record it in the array
Else
 
'loop through array to find free spot for next ip
For j = 1 To UBound(arrIPs)
If arrIPs(j) = "" Then arrIPs(j) = strIP: GoTo PrintIP
Next j
End If
 
Next i
PrintIP:
txtSimple = txtSimple & strIP & strwrap
 
skipwrite:
Loop
Close #1
End Sub

First: use indentations - code is difficult to read.
Second: you are searching from and writing to THE SAME ARRAY - arrIPs().

As far as I could understand your LOOKUP array should be different from that you need to populate ...

would the fact that it is the same array break the operation of it? i didn't think it would..

and i personally hate indents .. i like reading it all inline..it's also a extremely small chunk of code, so i didn't think it would be a problem reading.

but i will indent from now on cuz i guess most people like it better.. sorry about that..

thanks for responding.. but does ne1 have any clues to why this code won't work?

Don't your arrays start at 0? If so, you are missing them.

Also, you don't set the new array position as being +1 to the last one.
You should redim with the preserved option before you write to the array.

Start out like this:

Dim arrIPs() As Variant

set the first position as, say for example, z=0

when you go to add it

redim preserve arrIPs(z + 1)

you wouldn't need to loop to find the next open spot.

If strIP = arrIPs(i)

this is where my index was being incremented.. so actually i WAS adding one each time..


I've never used the preserve thing.. i guess i'll have to read up on it.. i don't really know to impliment it.

thanks though.

this is what i'm still getting..

xx-xxx-xxx-xxx
yy-yyy-yyy-yyy
yy-yyy-yyy-yyy
yy-yyy-yyy-yyy
yy-yyy-yyy-yyy

the first ip it comes across is never repeated.. but something is not working as far as my storing a new ip into the array...

ne other advice?


here is the problem code:

VB Code:

Do Until EOF(1)
    Line Input #1, strNewLine
    ' get the ip string from the start of the log
    strIP = (Mid(strNewLine, 1, 15))
'============================================== PROBLEM
    'loop through the array to see if the ip is in it
        For i = 0 To UBound(arrIPs)
            ' if it is, then go to the next line
            If strIP = arrIPs(i) Then
                GoTo skipwrite
                ' if not, then record it in the array
            Else
 
                'loop through array to find free spot for next ip
                For j = 0 To UBound(arrIPs)
                    If arrIPs(j) = "" Then
                        arrIPs(j) = strIP
                        txtSimple = txtSimple & strIP & strwrap
                        GoTo skipwrite
                    End If
                Next j
            End If
        
        Next i
'========================
 
 
skipwrite:
 
 
 
Loop
Close #1

Try something like this.

VB Code:

Dim arrPIs() As String
ReDim arrPIs(0)
Dim numIPs As Long
Dim Found As Boolean
 
Do Until EOF(1)
    Line Input #1, strNewLine
    ' get the ip string from the start of the log
    strIP = (Mid(strNewLine, 1, 15))
'============================================== PROBLEM
    'loop through the array to see if the ip is in it
    Found = False
    For i = 0 To numIps
        If strIP = arrIPs(i) Then
            Found = True
            Exit For
        End If
    Next i
 
    If Found = False Then
        arrIPs(numIPs) = strIP
        numIps = numIPs + 1
        ReDim Preserve arrIPs(numIPs)
        txtSimple = txtSimple & strIP & strwrap
    End If        
'========================
 
Loop
Close #1

I always set NumIPs to 0 right after declaring. It's an old habit that has saved me quite a few times. It will work this way, but only if the variable is declared and destroyed. For instance, if you need to know NumIPs somewhere else, you can't use it. If you make it a public variable, then you NEED to set it to 0 before you start the banning routine, or things will go awry quickly, and it will be very hard to figure out why.

hmm... that code looks really promising.. thanks!!! I can't wait to try it out.

I don't know why I don't just download a log parser.. but... i like to make things myself.. :p

Just wanted to mention that you don't have to declare and use your own "strwrap" since VB has done that already. Either use vbCrLf or vbNewLine.

Also adding text to the end of a text box is much faster then replacing all text:

VB Code:

txtSimple.SelStart = Len(txtSimple.Text)
txtSimple.SelText = strIP & vbCrLf

Quote:

---

Originally Posted by dglienna

I always set NumIPs to 0 right after declaring. It's an old habit that has saved me quite a few times. If you make it a public variable, then you NEED to set it to 0 before you start the banning routine, or things will go awry quickly, and it will be very hard to figure out why.

---

It's not necessary to initialise numIPs if it's private to the routine, which I suggest. You are quite correct about 'things going awry with public variables', that's why it's a good idea to use them sparingly. IMHO there are better (and safer) ways to pass and share data between forms/modules etc.

hmm.. thanks for the tips guys. I've been working on vb for a while now.. and I'll never know enough about it... is there any point where i become a professional? :)

Just keep pluggin away.... And don't be afraid to ask questions. You'll get there.

Good luck :thumb:

Sweet. Thanks for the encouragement.. I can use it for those days when i want to give it all up.. :)

P.S. I've been to Fremantle (fermantle?) and Perth.. love it.. can't wait to go back.

You are a professional when you have been paid to do some work, and properly initiated when you fixed a problem.. ;)