Sometimes people are programming who are just pretending to be programmers. Unfortunately it drives up costs of projects and makes maintenance a nightmare. Here is a very simple example of what an ex-coworker of mine wrote in this error handler comments. It should be obvious from his comment that he really had no idea what he was doing. Yet, he's been writing VB code for over 4 years now.
'******************************************************
' CheckFailed
'------------------------------------------------------
' if there is nothing in the bypass list, delete it
'******************************************************
Private Function CheckFailed()
On Error GoTo errhandler
Set oFailedFile = Nothing
If gFailedFlag = False Then
Kill (gFailedPathandName)
End If
errhandler:
'it's a minor function, and certain conditions will cause vb errors - so let it go
Resume Next
End Function
Think about it. What's the point of having an error handler if you're going to dismiss any errors and resume program execution. What possible "certain conditions" could he be talking about and how should he handle them? I see only a few, and they're all easy to handle. Assuming no compile errors and Option Explicit, there could only be a problem with the Kill (gFailedPathandName) statement. The filename is either invalid or the user doesn't have permissions to delete the file, or it is in use. Not much else to think about here. "So let it go"? This is a production function, meaning those files MUST be deleted once they are processed, or it creates a disk space leak.
Ugh. Gimme a break.
DaysBack = 0
If Not IsNumeric(gWeeksToKeep) Then
'default to no more than 10 weeks of logs and lists
gWeeksToKeep = 1
End If
For i = 1 To gWeeksToKeep
DaysBack = DaysBack + 7
Next i
Heh. Looks like something iago said he once did. =P
Quote from: Grok on December 04, 2003, 10:51 AM
Ugh. Gimme a break.
DaysBack = 0
If Not IsNumeric(gWeeksToKeep) Then
'default to no more than 10 weeks of logs and lists
gWeeksToKeep = 1
End If
For i = 1 To gWeeksToKeep
DaysBack = DaysBack + 7
Next i
mmmm fake multiplication
More example of programmers not knowing where they are or what they're doing:
For i = 0 To Val(LineCount - 1)
InputString = UCase$(ts.ReadLine)
For j = 0 To (gSearchPhraseCount - 1)
Found = CheckSomething()
If Found > 0 Then
'dostuff1
If OtherCheck = True Then
'dostuff2
Exit For
Else
'dostuff3
If AnotherCheck = True Then
'dostuff4
Exit For
Else
'dostuff5
Exit For
End If
End If
End If
Next j
'gPassOverFlag = False
If Found > 0 Then Exit For
Next i
Is it not totally obvious that he's going to Exit For no matter what happens after Found > 0?! Arggg @ sucky code.
More code indicating completely lost programmer:
'get the julian date
CompareDate = Format(Now(), "y")
If Val(CompareDate) < DaysBack Then
DateHold = 365 - (DaysBack - Val(CompareDate))
NewCompareDate = Format(DateHold, "mm-dd-yy")
Else
DateHold = Val(CompareDate) - DaysBack
NewCompareDate = Format(DateHold, "mm-dd-yy")
NewCompareDate = Mid(NewCompareDate, 1, 6) & Format(Now(), "yy") 'to handle vb bug
End If
His lack of comprehension of what he is doing causes him to blame VB for the last line of code being necessary. This comes from him having no idea how Format works with numbers and dates, so when he wants to format the julian day back to a date, it has no year, and thus its zero value becomes year 2000. He should've just used DateAdd and most of the above code would've been unnecessary:
NewCompareDate = Format(DateAdd("d", -DaysBack, Now()), "mm-dd-yy")
Are these all from the same guy/project?
YES. And OMG --- look at this code ... it's total purpose is to output a message. The reason it's so huge is his handling of singular/plural with "file" vs "files", and "was" vs "were"
If SkipFlag = True Then
If gTotalFilesProcessed = 1 And gTotalKickout = 1 Then
Msg = "Finished!" & vbCrLf & gTotalFilesProcessed & " file was processed!" & vbCrLf & gTotalSuccessful & " succeeded, and " & gTotalKickout & " was bypassed." & vbCrLf
Else
If gTotalKickout = 1 Then
Msg = "Finished!" & vbCrLf & gTotalFilesProcessed & " files were processed!" & vbCrLf & gTotalSuccessful & " succeeded, and " & gTotalKickout & " was bypassed." & vbCrLf
Else
If gTotalFilesProcessed = 1 Then
Msg = "Finished!" & vbCrLf & gTotalFilesProcessed & " file was processed!" & vbCrLf & gTotalSuccessful & " succeeded, and " & gTotalKickout & " were bypassed." & vbCrLf
Else
Msg = "Finished!" & vbCrLf & gTotalFilesProcessed & " files were processed!" & vbCrLf & gTotalSuccessful & " succeeded, and " & gTotalKickout & " were bypassed." & vbCrLf
End If
End If
End If
Msg2 = Msg & "See log for details."
msg3 = Msg2
Msg2 = "_____________________________________________________" & vbCrLf & vbCrLf & Msg & vbCrLf & "_____________________________________________________"
WriteLog (Msg2)
Info Msg2, vbRed
Info "", vbRed
Beep
MsgBox msg3
Else
If optReprocess.Value = True And Len(gDirToProcess(0)) = 0 Then
Msg = "There are no files to process at this time!"
MsgBox Msg
Info Msg, vbRed
Else
If gTotalFilesProcessed = 1 And gTotalKickout = 1 Then
Msg = "Finished!" & vbCrLf & gTotalFilesProcessed & " file was processed!" & vbCrLf & gTotalSuccessful & " succeeded, and " & gTotalKickout & " was bypassed." & vbCrLf
Else
If gTotalKickout = 1 Then
Msg = "Finished!" & vbCrLf & gTotalFilesProcessed & " files were processed!" & vbCrLf & gTotalSuccessful & " succeeded, and " & gTotalKickout & " was bypassed." & vbCrLf
Else
If gTotalFilesProcessed = 1 Then
Msg = "Finished!" & vbCrLf & gTotalFilesProcessed & " file was processed!" & vbCrLf & gTotalSuccessful & " succeeded, and " & gTotalKickout & " were bypassed." & vbCrLf
Else
Msg = "Finished!" & vbCrLf & gTotalFilesProcessed & " files were processed!" & vbCrLf & gTotalSuccessful & " succeeded, and " & gTotalKickout & " were bypassed." & vbCrLf
End If
End If
End If
i dont do a whole ton of vb for my boss so my knowledge on the subject is vague..but.. Do most vb programmers make stupid mistakes from laziness? If yes, im glad he wasnt a net tech..ex.
"Well for some reason these stations cant see the file server, ill just mark these computer bad"
laziness in my field costs uber cash, i wish people would just get over their self pride and ask about something they didnt know, rather then improvise and make my job harder then it needs to be.
Setting the maximum value of a progress bar:
Select Case gPreProcessLineCount
Case Is > 100000
PBar2.Max = 250000
Case 50000 To 100000
PBar2.Max = 200000
Case 10000 To 49999
PBar2.Max = 150000
Case 5000 To 9999
PBar2.Max = 100000
Case 2000 To 4999
PBar2.Max = 50000
Case Else
PBar2.Max = 10000
End Select
It's worth noting that the maximum value range of a progress bar is -32,768 to 32,767. if he sets it to 50000, it will raise an error, but that's OK, his error handler is RESUME NEXT.
Quote from: Grok on December 08, 2003, 09:21 AM
YES. And OMG --- look at this code ... it's total purpose is to output a message. The reason it's so huge is his handling of singular/plural with "file" vs "files", and "was" vs "were"
That's where you use IIf?
Quote from: Grok on December 08, 2003, 10:14 AM
Setting the maximum value of a progress bar:
...
It's worth noting that the maximum value range of a progress bar is -32,768 to 32,767. if he sets it to 50000, it will raise an error, but that's OK, his error handler is RESUME NEXT.
Other than that, the progress bar range setting code was pretty OK?
Absolutely not. The implementation really has nothing to do with the real progress. Every time his Value goes over the Maximum, he resets Value and bumps up Maximum, or something. I haven't really figured out what he's doing but based on watching it run, he'd be better off with a "busy" indicator than a progress bar. He should use a progress bar to indicate how far through the file list he has gone.
The code is thousands of lines of spaghetti which could be reduced to about 20% of its current size. I just don't have the time to fix it, but am forced to step through it for a current production problem.
Might I also add that 'gPreProcessLineCount' variable gets it value from FileLen(ProcessFileName), and not the line count. That might shed some light on why the Select Case is nutty.
Ahh.. I thought perhaps he had a bunch of these scales and was setting them all to something that could be compared. But if he's changing the scale as things progres..... Hmm, sounds like Internet download progress bars :P