How to tell when programmers are guessing

[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 Select Expand


'******************************************************
' 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.

[Grok]

Ugh.  Gimme a break.

Code Select Expand

   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


[Spht]

Heh. Looks like something iago said he once did. =P

[Banana fanna fo fanna]

Ugh.  Gimme a break.

Code Select Expand

   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

[Grok]

More example of programmers not knowing where they are or what they're doing:

Code Select Expand


   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.

[Grok]

More code indicating completely lost programmer:

Code Select Expand


   '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:

Code Select Expand

   NewCompareDate = Format(DateAdd("d", -DaysBack, Now()), "mm-dd-yy")

[hismajesty]

Are these all from the same guy/project?

[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 Select Expand


   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


[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.

[Grok]

Setting the maximum value of a progress bar:

Code Select Expand


   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.

[Adron]

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?

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?

[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.

[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