|
Exception-driving development, my dear Brady!
Religiously blogging on the intarwebs since the early 21st century: Kineti L'Tziyon
Judah Himango
|
|
|
|
|
It is just a JIT compiler test. A "good" compiler would deduce the intent and eliminate all of the excess excpetion handling and place optimized code inline on the first pass. This eliminates the need for the programmer to think because the compiler has implemented the new DWIM (Do what I mean) facility.
Seriously in MVS Assembler it would be
XR R0,R0
L R1,VALUE
D R0,=A(2)
JZ EVEN
process odd number
J M_010
EVEN DS 0H
process even number
M_010 DS 0H
.
.
.
Sam
|
|
|
|
|
0. This code is in the region of 10 years old and I only had VB (probably 5 at that point) to work with.
1. What the code does is, I think sound.
In each class these variables are defined:
Private mlErrNumber As Long '** Standard Variable for Error Trapping **
Private msErrDescription As String '** Standard Variable for Error Trapping **
Private msErrSource As String '** Standard Variable for Error Trapping **
Then in any function or sub mlErrNumber = 0 clears the error state.
Not bad so far, slightly ott, but not bad.
So when an error occured, this was called to persist the error state to be used by any calling classes:
Private Sub SetError(ByVal alNumber As Long, _
ByVal asDescription As String, _
ByVal asSource As String, _
ByVal asLocation As String)
'================================================================================================
' Sub : SetError
' Arguments : alNumber Number of the error
' asDescription Description of the error
' asSource Source of the original error (Added 1.00.0001)
' asLocation Location of the error
'------------------------------------------------------------------------------------------------
' About : Sets the current error.
' If mlErrNumber is already set, it just appends to the location
'================================================================================================
On Local Error Resume Next
If mlErrNumber = 0 Then
mlErrNumber = alNumber
msErrDescription = asDescription
If asSource = APP_TITLE Or _
asSource = asLocation Or _
asSource = "" Then
msErrSource = ""
Else
'It's someone else's error, record where it came from
msErrSource = " " & asSource
End If
Call gObjCommon.SendToLog("E", mlErrNumber, 1, CLASS_NAME, asLocation, _
msErrDescription & IIf(msErrSource = "", "", " [Occured in" & msErrSource & "]"))
End If
If asLocation <> "" Then
msErrSource = "." & asLocation & msErrSource
End If
End Sub
The horror?
Appart from the language, why didn't I see this as being repeated? It's in ~50 seperate classes!
Panic, Chaos, Destruction.
My work here is done.
|
|
|
|
|
VB kind of forced all of us to a number of little implementation horrors, what with fake OOP and all.
I had my share of VB back in the day, and I'm happy it's over! When I have to maintain some of my old code from the 90's I still shiver at some horrors I see. It sure was partly my fault, but I'm convinced VB did its part.
We will probably enter yet another phylosophical/religious debate if we start arguing about goods and bads in old VB, but I think (hope) everybody will agree that even if it did have a sense in its day, it was just EVIL.
2+2=5 for very large amounts of 2
(always loved that one hehe!)
|
|
|
|
|
I have a little timer class that i use in optimizing code. It's a simple thing:
class CISTimer
{
public:
CISTimer() {m_p = NULL; m_t = GetTickCount();}
CISTimer(const char *p) {m_p = p; m_t = GetTickCount();}
~CISTimer()
{
DWORD d = GetTickCount() - m_t;
char buf[30];
_snprintf(buf, 29, "%4.2f (%d)\n", (double)(d) / 1000.0, d);
if (m_p) {OutputDebugString(m_p);}
OutputDebugString(buf);
}
protected:
const char *m_p;
DWORD m_t;
};
grabs the time when it's created and outputs the elapsed time when it's destructed.
so, i'm working on a new project and want to see how long certain chunks of code are taking. i put the function calls inside curly braces, and drop some counters in:
{
CISTimer("Func1");
Func1(...);
}
...
{
CISTimer("Func2");
Func2(...);
}
so the counters will print their elapsed time when they go out of scope, thus telling me how long the function took, so i'll know where to start optimization.
but they're all coming up with an elapsed time of 0. that can't be true.
so i switch from GetTickCount to QueryPerformanceCounter. but, all those come up with 15 ticks - far too fast. the calls should be in the .25 second range.
so, i step through the code. the breakpoint gets to the counter instantiation, i step over it, and the timer immediately destructs - before going out of scope. did i break C++'s deterministic destruction?
after much head-scratching, i realized that i forgot to give names to those CISTimer objects. they were anonymous, and so they disappeared as soon as they were created. duh.
this is an unpleasant feature of C++.
|
|
|
|
|
Why do you classify this as a coding horror?
I would say this is more like a subtle bug...
|
|
|
|
|
there is no "subtle bug" forum. but the intro text of this forum includes the category "embarrasing mistakes" (which, amusingly, is a spelling mistake).
|
|
|
|
|
Chris Losinger wrote: there is no "subtle bug" forum
There is such a forum. It was called 'subtle bugs' for a long time and was just recently renamed to 'Wicked Code'. See here: http://www.codeproject.com/Feature/WickedCode.aspx.
Regards
Thomas
www.thomas-weller.de
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.
|
|
|
|
|
again, the intro text to this forum reads:
We all come across code that simply boggles the mind. Lazy kludges, embarrasing mistakes, horrid workarounds and developers just not quite getting it.
if you feel that is in error, there is a Suggestions forum, too.
modified on Monday, June 1, 2009 7:11 AM
|
|
|
|
|
|
So you think (compiler) optimization is unpleasant? 
|
|
|
|
|
i think allowing anonymous variables outside the context of a function call is unpleasant.
|
|
|
|
|
I'm not sure what you mean by anonymous variables. It is quite normal in C to have expressions yield a value which is ignored. Even "intvar=5" is an expression which yields the value five. It is also quite common to have functions return a value which is ignored. To be sure, there are places where return values shouldn't be ignored but are anyway; on the other hand, it would be annoying if one always had to explicitly use the return value of something like memcpy() instead of just letting it vanish into the ether.
|
|
|
|
|
supercat9 wrote: I'm not sure what you mean by anonymous variables.
anonymous = "without a name".
supercat9 wrote: It is quite normal in C to have expressions yield a value which is ignored.
right. but that has nothing to do with the code i posted above. this isn't about return values.
in C++ you can do this:
CPoint(5,15);
that will construct a CPoint object and immediately destruct it because the variable is anonymous, it has no name, and it isn't used anywhere.
you can also do this:
MyFunc(CPoint(5,15));
this will create a CPoint and pass it to MyFunc.
the latter case us useful - and it's very common to do this when you need to re-package some data that you already into the types that the function expects.
but the former case is much less useful. if the c'tor and d'tor have side-effects, you could possibly use that to take advantage of those side-effects. but it's very non-intuitive. you would be better off moving those side effects to a function that you could call explicitly.
|
|
|
|
|
this will create a CPoint and pass it to MyFunc.
A constructor is essentially a function call. The function returns a value, which may be used like any other rvalue. The expression "somevariable = CPoint(5,15)" has a value which would typically be ignored, but could be used as part of a larger expression. The compiler doesn't particularly care whether or not it makes sense for the result of a particular expression to be ignored; while one typically ignore the value of an assignment expression, and would not typically ignore the value of a constructor, there are situations where one might very reasonably want to do the opposite of the usual case. One may, for example, wish to follow a 'next' pointer and check whether it points anywhere, or force the early creation of a singleton object one is not immediately interested in using.
|
|
|
|
|
i'm not sure why you refuse to acknowledge my point.
supercat9 wrote: The compiler doesn't particularly care whether or not it makes sense for the result of a particular expression to be ignored
i'm not complaining about what the compiler does. i'm complaining about the fact that the language allows such a thing to happen.
it makes no sense at all to construct an anonymous object outside of the context of a function call.
this makes sense:
CString t1 = CString ("sdfs");
this also makes sense:
MyFunc(CString ("sdfs"));
this makes no sense:
CString ("sdfs");
|
|
|
|
|
Chris Losinger wrote: i'm not sure why you refuse to acknowledge my point.
I am.
--
Kein Mitleid Für Die Mehrheit
|
|
|
|
|
Or, "How not to do your release notes.":
str3 = str3 + "<tr><td align='center'><b> Version 2.1.101 • 15 Sep 2007 </b></td></tr>" +
"<tr><td> </td></tr>" +
"<tr><td> </td>• Resolved script error on Reports Viewed List Report</tr>" +
"<tr><td> </td>• Resolved error when adding level using MSSQL</tr>" +
... [snip]
"<tr><td> Corrected Directory Listing query to display Extension Number of Parent Extension </td></tr>" +
"<tr><td> </td></tr>"; 730 lines have been removed from the string literal to protect everyone, not just the innocent.
You really gotta try harder to keep up with everyone that's not on the short bus with you.
- John Simmons / outlaw programmer.
|
|
|
|
|
Wow... just wow.
As me and my brother debate about regularly, at what point would a 'normal' person stop, take a step back, and say "really... wtf am i doing?". And what should we do with the non-normal people that either keep going for a REEEEEAAAALLLLYY long time, or worse yet, never hit that mark?
From both our experience, it would appear to be - make them CTO.
-------------------------------
Carrier Bags - 21st Century Tumbleweed.
|
|
|
|
|
So not only did he use over 700 unnecessary string concatenations, but he couldn't even put the text INSIDE the table cells?
I must say I'm impressed... It takes real talent to make a coding horror that spans two languages! (Three, if you count English -- Extension Number of Parent Extension? Is that from the Department of Redundancy Department?)
|
|
|
|
|
That's highly inefficient too. This is much more efficient:
str3 = "<tr><td align='center'><b> Version 2.1.101 • 15 Sep 2007 </b></td></tr>"
"<tr><td> </td></tr>"
...
"<tr><td> </td></tr>";
That way, the compiler will be able to do the string joins at compile time, rather than at runtime using the overloaded + operator. I'm assuming this is C++ and not C#. The initial str + would not have been needed in C#...
--
Kein Mitleid Für Die Mehrheit
|
|
|
|
|
It's decompiled C#.
You really gotta try harder to keep up with everyone that's not on the short bus with you.
- John Simmons / outlaw programmer.
|
|
|
|
|
Whoa.. It didn't even optimize the literals when it was compiled? Did someone forget to turn on the optimizer?
--
Kein Mitleid Für Die Mehrheit
|
|
|
|
|
I found this function in the service I'm having to debug. I've omitted a few hundred lines of the code so that you may more clearly see the control flow.
void ProcessMessage()
{
if ( )
{
}
if ( )
{
}
if ( )
{
}
if ( )
{
}
while ( )
{
if ( )
{
continue;
}
if ( )
{
continue;
}
if ( )
{
continue;
}
if ( && )
{
if ( || )
{
if ( )
{
}
else
{
}
continue;
}
}
if ( )
{
continue;
}
for ( ; ; )
{
if ( )
{
}
if ( )
{
if ( )
{
}
continue;
}
if ( )
{
if ( )
{
}
continue;
}
if ( )
{
if ( )
{
}
else
{
}
if ( || || ( && ))
{
if ( )
{
}
}
else
{
if ( )
{
}
}
continue;
}
if ( )
{
for ( ; ; )
{
if ( )
{
continue;
}
if ( )
{
if ( )
{
}
else
{
}
if ( )
{
}
break;
}
else if ( )
{
if ( || )
{
if ( )
{
if ( )
{
}
}
}
if ( )
{
}
else
{
}
continue;
}
else
{
if ( )
{
}
break;
}
}
}
else
{
if ( )
{
}
}
if ( )
{
if ( )
{
}
}
else
{
for ( ; ; )
{
}
}
if ( || || )
{
if ( )
{
}
else if ( )
{
}
else
{
}
if ( )
{
}
else
{
if ( )
{
}
else
{
}
if ( || ( && ))
{
if ( )
{
if ( )
{
}
else
{
}
}
}
else
{
if ( )
{
if ( )
{
}
else
{
}
}
}
}
}
}
for ( ; ; )
{
if ( )
{
}
}
if ( )
{
if ( )
{
}
if ( )
{
if ( )
{
}
}
else
{
if ( )
{
}
}
}
else
{
}
}
}
|
|
|
|
|
add a couple of gotos and it'll be perfect
|
|
|
|