|
Same here. I have written applications that have been online for months without any problems and they whine about the time it took to get them to work that well. The old application fell apart every day and was almost unmaintainable, but this time and effort somehow does not count.
"Dark the dark side is. Very dark..." - Yoda
--- "Shut up, Yoda, and just make yourself another toast." - Obi Wan Kenobi
|
|
|
|
|
Ran across this little gem while cleaning some old (but still in active development ) VB6 code today:
If GV_GracefullyExiting Then
End
End
End
End If
* Names and dates removed to protect the ???
Was it really that hard to track down why the application wouldn't die that it needed to be ended three times? I mean, a double-tap is standard zombie killing procedure, but thrice?
|
|
|
|
|
Yeah, and those other two End statements should be commented! Like so:
If GV_GracefullyExiting Then
End
End
End
End
End
End
End
End If
|
|
|
|
|
If GV_GracefullyExiting Then
While True
End
End While
End If It is VB after all - the Language That Would Not Die...
Real men don't use instructions. They are only the manufacturers opinion on how to put the thing together.
Manfred R. Bihy: "Looks as if OP is learning resistant."
|
|
|
|
|
|
I wish I could do more than 5
|
|
|
|
|
Yet Another VB Horror!
The following is an error-handling block from some old code I'm having to maintain (for my sins)..
errorPAT:
If retryCount < 1000 Then
ElseIf Err.Number = errOdbcCallFailed Then
If InStr(odbcErrorMess(Err), "insert duplicate") > 0 Then
retryCount = retryCount + 1
Resume tryAgain
ElseIf InStr(odbcErrorMess(Err), "unique constraint") > 0 And InStr(odbcErrorMess(Err), "violated") > 0 Then
retryCount = retryCount + 1
Resume tryAgain
ElseIf InStr(odbcErrorMess(Err), "Duplicate key value specified") > 0 Then
retryCount = retryCount + 1
Resume tryAgain
Else
message "Error " & Format(Err, "######") & " " & odbcErrorMess(Err), "P_ArchiveTag"
Exit Sub
End If
ElseIf Err.Number = errDuplicateValues Then
retryCount = retryCount + 1
Resume tryAgain
End If
message "Error " & Format(Err, "######") & " " & Error, "P_ArchiveTag"
End Sub
I think there's ample horrors for a short chunk of code...
|
|
|
|
|
It's 8am here. Remind me to never again visit this forum before breakfast. I can see this ruining my whole day (and no, I'm not going to sea, but that's another story.)
:vomit:
Peter
Software rusts. Simon Stephenson, ca 1994.
|
|
|
|
|
Yes, like the unique constraint in there, I feel "violated"
|
|
|
|
|
There a very aptly named method/function called 'odbcErrorMess', which neatly sums the whole code block you pasted.
Take-away: never shorten your method/function names
'As programmers go, I'm fairly social. Which still means I'm a borderline sociopath by normal standards.' Jeff Atwood
|
|
|
|
|
Julien Villers wrote: Take-away: never shorten your method/function names
I don't know, in this case I'd say the variable name is spot on .
Must be the only example in the code of a well named variable. I have such treasures as Function names:
Function H_WriteRecord1()
Function H_WriteRecord2()
Function H_WriteRecord3()
Note - those are the declarations as they appear. No return type given, all state passed in via global or module-level variables, and nothing to indicate what gets written, what the difference between the three variants is. I suspect "that nearly does what I want, I'll copy and paste it".
I also like the "batchHeader" boolean that sometimes outputs a header when true, otherwise when false. I still can't work out the logic governing when the two forms are used.
I suspect the originator may be a closet worshipper of The FSM[^]. I just wished he hadn't felt the need to spread noodly appendages through the code I'm now tasked with maintaining.
|
|
|
|
|
I was assigned to do some changes on built page and I found this.....
protected void btnEditClick(object sender, EventArgs e)
{
for (int i = 0; i < gvContactInfo.Rows.Count; i++)
{
var row = gvContactInfo.Rows[i].Cells[8];
ImageButton button = row.FindControl("modifyLink") as ImageButton;
if (button.GetHashCode() == SenderButton.GetHashCode())
{
ContactInfoView contactInfoView = row.FindControl("ctlContactInfoView") as ContactInfoView;
ctlNewContactInfo.ContactInfo = contactInfoView.ContactInfo;
}
}
}
|
|
|
|
|
Why? Why?
Why check an already boolean result?
Noticed this redundancy in many parts of an ill written app.
If myControl.Visible = True Then
Else
End If
I don't know why, it's just plain turn off to see this redundancy!
-
Just that something can be done, doesn't mean it should be done. Respect developers and their efforts!
Jk
|
|
|
|
|
Unfortunately this is the biggest issue I have about vb. It's too easy to be deep in thought and literally code what you are thinking and make the code more annoying to read. i.e. If control x visible property is true then do this and this and this. I usually pick it up after I have written it and clean it up afterwards but sometimes I've come across code a few months later and go whoops.
Hopefully the compiler is smart enough to fix the extra redundancy I've added so it doesn't effect performance.
|
|
|
|
|
I don
Anyway, to be really sure the control is visible you should obviously write
<pre>If (myControl.Visible = True) = True Then...
> so it doesn't effect performance
You mean "affect" (I hope)
|
|
|
|
|
And I ran this test once with "if mybool" and once with "if mybool = True".
No difference in speed. 1st one showed 33 seconds, 2nd one showed 32 seconds.
Module Module1
Sub Main()
Dim mytime As Date
Dim mybool As Boolean
Dim mydiff As TimeSpan
Dim j As Double
mytime = TimeOfDay()
While mytime = TimeOfDay()
End While
mybool = True
j = 0
For i = 1 To 100000
For j = 1 To 100000
If mybool = True Then
j = j + 1
End If
Next
Next
mydiff = TimeOfDay() - mytime
Console.WriteLine(mydiff)
Console.ReadKey()
End Sub
End Module
|
|
|
|
|
Had you taken a look at the disassembly, you would have discovered that the compiler generates exactly the same code in both cases. There is no real need for a test program.
"Dark the dark side is. Very dark..." - Yoda
--- "Shut up, Yoda, and just make yourself another toast." - Obi Wan Kenobi
|
|
|
|
|
Ummm...if you dissassemble something isn't there a program to disassemble? Ergo a test program?
I was simply providing an example that proves that this is not redundant code at all. The disassembly does confirm it though so thanks for that observation.
|
|
|
|
|
What I meant is, that when such a question arises, I simply write both lines in the application I'm working on and then look what I find in the disassembly. Why try to measure something that you can examine directly? As to the topic: I see that just as you do. It's not redundant and has no impact (in all languages I commonly use). A difference which makes no difference is no difference.
"Dark the dark side is. Very dark..." - Yoda
--- "Shut up, Yoda, and just make yourself another toast." - Obi Wan Kenobi
|
|
|
|
|
mdblack98 wrote:
Dim j As Double
...
j = 0
For j = 1 To 100000
...
j = j + 1
...
Next Ouch!
* Declaring a control variable as a Double
* Redundant initialisation of a variable immediately before using it as a control variable
* Doing integer arithmetic on a Double (in the For and in the assignment statements)
* Changing the control variable inside the loop
* Writing a Hall Of Shame contender in response to a Hall Of Shame entry. Priceless!
|
|
|
|
|
|
I actually sometimes check is some condition is not fulfilled even when there is else branch.
This I do in cases when one case of if/else block is expected behavior and in other one i do simple logging/recovery or similar thing.
Specifically, I do this when one block is short (less than 5 lines), and put that block in front. In that case, code is more clean to my eye because else block is very near to if block. This is very helpful to me when i have several nested if/else structures.
Would that make sense to anyone but me?
|
|
|
|
|
makes sense to me to have the preferred condition as the first block of code
if not WingsFallenOff then
FlyNormally
else
Panic
End if
|
|
|
|
|
How about:
try{
if (wingsFallenOff) {
throw new OutOfLiftError("Ahhhhhhhhhhh!!!!!");
}
flyNormally();
} catch (OutOfLiftError ex) {
panic(ex);
}
To be honest, rather then checking the boolean value, a method call, say checkAirworthy() , could be used that, if the plane is in the air, will throw an exception rather than returning false . Thta means that you can later upgrade it to check for engines, air-con and lemon-scented hand sanitisers. The same method would then be used before pushing back.
Panic, Chaos, Destruction. My work here is done.
Drink. Get drunk. Fall over - P O'H
OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre
I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer
Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett
|
|
|
|
|
It may seem structurally sound, but it's a mistake to use exception handling for control flow.
First, exceptions are exactly that. Cases which you do not expect should happen in normal workflow, exceptional situations which are caused by a failure in the system. They should not be used to control business logic. However, I am also tempted to use them in this way when I have more than 2 layers of abstraction between business logic decision, and handling of that decision.
Second, it's big hit to performance. If this is not client side code, or a single occurrence during request processing, this can be real PITA when you come to a stage you need to optimize performance.
|
|
|
|