Valhalla Legends Forums Archive | Visual Basic Programming | How to tell when programmers are guessing

AuthorMessageTime
Grok
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.

[code]
'******************************************************
' 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
[/code]

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.

December 4, 2003, 1:50 PM
Grok
Ugh. Gimme a break.

[code] 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
[/code]
December 4, 2003, 4:51 PM
Spht
Heh. Looks like something iago said he once did. =P
December 4, 2003, 5:30 PM
St0rm.iD
[quote author=Grok link=board=31;threadid=4072;start=0#msg33631 date=1070556703]
Ugh. Gimme a break.

[code] 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
[/code]

[/quote]

mmmm fake multiplication
December 4, 2003, 9:23 PM
Grok
More example of programmers not knowing where they are or what they're doing:

[code]
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
[/code]

Is it not totally obvious that he's going to Exit For no matter what happens after Found > 0?! Arggg @ sucky code.
December 4, 2003, 9:30 PM
Grok
More code indicating completely lost programmer:

[code]
'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
[/code]

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:

[code] NewCompareDate = Format(DateAdd("d", -DaysBack, Now()), "mm-dd-yy")[/code]
December 8, 2003, 2:48 PM
hismajesty
Are these all from the same guy/project?
December 8, 2003, 2:59 PM
Grok
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"

[code]
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
[/code]
December 8, 2003, 3:21 PM
ObsidianWolf
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.
December 8, 2003, 3:30 PM
Grok
Setting the maximum value of a progress bar:

[code]
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
[/code]

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.
December 8, 2003, 4:14 PM
Adron
[quote author=Grok link=board=31;threadid=4072;start=0#msg34271 date=1070896905]
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"
[/quote]

That's where you use IIf?


[quote author=Grok link=board=31;threadid=4072;start=0#msg34278 date=1070900075]
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.
[/quote]

Other than that, the progress bar range setting code was pretty OK?
December 8, 2003, 5:16 PM
Grok
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.
December 8, 2003, 5:30 PM
Adron
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
December 8, 2003, 5:37 PM

Search