Author | Message | Time |
---|---|---|
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 |