|
I used to have a co-worker who did almost the same
It went a bit along the lines off this: (sorry for the vb code but well I code in it  )
Dim rc as integer=0
rc = doSomething()
if rc <> 0 then goto Errormessage 'yeah a goto
rc= = dosomethingelse()
if rc <> 0 then goto Errormessage
... ' went on and on like this for about 200 lines
Errormessage:
Select case rc
case 1
messagebox.show("...")
case 2
messagebox.show("...")
...
end select
I true nightmare to debug but it wasn't even the worst thing I saw in his code. O and I should mention that we had/have a specific way to handle errors and the messages that should go with them.
Needless to say this isn't that way , he just choose to ignore everything we(manly my boss (and his)) told him and just do his own thing, he didn't last very long.
If I have the time I'll post some of the horrors I'v seen in it
|
|
|
|
|
I don't see the problem here (I assume this is VB 6?). It's a linear sequential pattern. If you think about it it's semantically equivalent to a try-catch block.
Kevin
|
|
|
|
|
If it was VB6 is would agree but this code was written in .NET and only about a year ago (with visual studio 2005)
|
|
|
|
|
OK, in that case he should be shot. 
BTW, why did they keep that stuff in VB .NET? Ditto Option Explicit turned off by default?
Kevin
|
|
|
|
|
Yeah and that wasn't even his biggest horror  I'd laugh if I didn't have to work in the code
|
|
|
|
|
This code was written by someone who is used to VB6, I bet.
In this case it's not the code that is awesome - it's the language...
Regards
Thomas
Programming today is a race between software engineers striving to build bigger and better idiot-proof programs, and the Universe trying to produce bigger and better idiots. So far, the Universe is winning.
Programmer - an organism that turns coffee into software.
|
|
|
|
|
Actually his resume said he had 20+ years of freelance programming experiance and 5 off those in C#.NET (if that is true is another story).
He also traded in a 3000€+ a month job for a 2000€ a month job 
|
|
|
|
|
Tom deketelaere wrote: Actually his resume said he had 20+ years of freelance programming experiance and 5 off those in C#.NET
Impossible. This is an old school VB programmer. You can clearly see this from the above snippet (I'm not saying it's somehow bad VB6 code, but that it for sure is VB6 code).
There must be sth. wrong here...
Regards
Thomas
Programming today is a race between software engineers striving to build bigger and better idiot-proof programs, and the Universe trying to produce bigger and better idiots. So far, the Universe is winning.
Programmer - an organism that turns coffee into software.
|
|
|
|
|
Well I can imagine that at some point he did work with vb6 (hell even I did) but this was written in .NET.
And his resume did say 5 years experiance in C#.NET but we (my boss and I) do think he had been lying a bit, since after he was fired here he told one off our customers (where he was being interviewed for a hardware maintenance position) he had experiance in maintaining servers.
He could barly keep his desktop pc running while he was here so... (he was actually not allowed to come anywhere near our servers :-P )
The guy had the tendancy to overcomplicated things (currently working on another one off his programs to get the CPU usage and Ram usage down since it olmost takes 100% CPU and more than 100mb ram, for a programme without GUI that's alot)
|
|
|
|
|
If the exceptions are forbidden for whatever reason, there is nothing wrong with it.
|
|
|
|
|
|
I agree. I generally prefer to see the normal paths first followed by the error paths. Deep nesting should be avoided other things being equal. But if there is a regular pattern to the code (as in your COM example elsewhere in the thread) this is fine. What is not fine is when you have deep nesting combined with loops and randomly sprinkled if-then-else blocks.
Kevin
|
|
|
|
|
Kevin McFarlane wrote: deep nesting combined with loops and randomly sprinkled if-then-else blocks.
They also call it reality - at least very often... 
Programming today is a race between software engineers striving to build bigger and better idiot-proof programs, and the Universe trying to produce bigger and better idiots. So far, the Universe is winning.
Programmer - an organism that turns coffee into software.
|
|
|
|
|
Kevin
|
|
|
|
|
Thats a messy "spaghetti code". It looks very strange or like C#. 
I would write more compact functions like LogOnHost(host,user,password) and SetFilePath(path,file)
Greetings from Germany
|
|
|
|
|
KarstenK wrote: It looks very strange or like C#.
What's your matter with C#? It allows for (and encourages) the cleanest and well structured code ever.
Regards
Thomas
|
|
|
|
|
But it allows -as seens- also the opposite.
Strange is in the above code alway one Set-Function for User and Password. I hope the Ftp-objects isnt a C# class. 
Greetings from Germany
|
|
|
|
|
KarstenK wrote: But it allows -as seens- also the opposite.
Sure, it allows for bad coding style as well - as every other programming language does even more.
Do you really blame C# for being a language that does not impose on its user what to say with it??
Do you have a car that forces you to adhere to traffic rules?
Does your money tell you what to buy with it?
...
Regards
Thomas
|
|
|
|
|
Right - and think about the possibilties of "bad coding" e.g. in C++ (add some mystic macros - done) vs. C#.. I think C# is a lot cleaner and full of nice features. The best programming language I know... (I used one or two during my career...  )
|
|
|
|
|
Where I am working, there USED to be a guy here who had written the following piece of code (swear to God this is a true function):
Private Function CaseInsensiveCompare(ByVal str1 As String, ByVal str2 As String) As Boolean
Dim Apples As String = str1.ToUpper()
Dim Oranges As String = str2.ToUpper()
Dim AreTheyEqual As Boolean = str1.Equals(str2)
Dim NotEqual As Boolean = Not AreTheyEqual
If AreTheyEqual = True Then
Return AreTheyEqual <> NotEqual
Else
Return AreTheyEqual = NotEqual
End If
Return (Not (AreTheyEqual)) = NotEqual Or AreTheyEqual
End Function
What boogles me is:
1. Why write a function for this? Just use If str1.ToUpper()=str2.ToUpper() Then ...
2. Why in the hell use this NOT logic stuff?
3. Is that final Return ever even hit in any scenario?
My best guess as to why he would have done this is maybe he was testing something and was commenting/decommenting a line in there.
|
|
|
|
|
This is a true horror & unfortunately I've seen my fair share of this 'style' of coding, usually from ex VB6 programmers... get over it & learn your way around the .net framework is what I say!
What's the deal with Apples & Oranges? they're not even used?? Are there other fruit themed vegetables variables in the project?!
"An eye for an eye only ends up making the whole world blind"
|
|
|
|
|
Yes, a horror indeed. Perhaps he wrote the code on April Fools Day
"The clue train passed his station without stopping." - John Simmons / outlaw programmer
"Real programmers just throw a bunch of 1s and 0s at the computer to see what sticks" - Pete O'Hanlon
"Not only do you continue to babble nonsense, you can't even correctly remember the nonsense you babbled just minutes ago." - Rob Graham
|
|
|
|
|
 The programmer is trying to compare Apples to Oranges. That function will always return false. 
Chris Meech
I am Canadian. [heard in a local bar]
In theory there is no difference between theory and practice. In practice there is. [Yogi Berra]
|
|
|
|
|
Youre completly wrong: the code will work. But not always return the truth.
CaseInsensiveCompare( "apple", "apple" ) -> true: OK
CaseInsensiveCompare( "apple", "orange" ) -> false: OK
CaseInsensiveCompare( "Apple", "apple" ) -> false: BUG
really SCNR 
Greetings from Germany
|
|
|
|
|
It gets worse. He's getting the upper case values, then comparing the original values
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
