|
Everything is between the if ( ) 
|
|
|
|
|
Uuuh... 
So what's the bug?
Wout
|
|
|
|
|
It's not a bug, but the code is very difficult to understand, even for experienced programmers. The guy should have used variables to store intermediate results.
|
|
|
|
|
Aaaah, I thought it was my unfamiliarity with linq, geez!
Wout
|
|
|
|
|
I swear I'm not making this up:
Private Sub aControl_DblClick()
'+++ VB/Rig Begin Push +++
Const VBRIG_PROC_ID_STRING = "+aControl_DblClick"
Dim VBRigErr As Long, VBRigErrMsg As String
If VBRig.Trap_TrapsEnabled Then
On Error GoTo aControl_DblClick_VBRigErr
End If
Call VBRig_Error(VBRIG_PUSH_PROC_STACK, 0, "", VBRIG_MODULE_ID_STRING, VBRIG_PROC_ID_STRING)
'+++ VB/Rig End +++
If someIndicatorVariable < 1 Then
'+++ VB/Rig Begin Pop +++
Call VBRig_Error(VBRIG_POP_PROC_STACK, 0, "", VBRIG_MODULE_ID_STRING, VBRIG_PROC_ID_STRING)
'+++ VB/Rig End +++
Exit Sub
End If
Call LaunchAnEXE("EXENAME", "params")
Unload Me
'+++ VB/Rig Begin Pop +++
Call VBRig_Error(VBRIG_POP_PROC_STACK, 0, "", VBRIG_MODULE_ID_STRING, VBRIG_PROC_ID_STRING)
Exit Sub
'=========================
aControl_DblClick_VBRigErr:
'=========================
VBRigErr = Err
VBRigErrMsg = Error$
Call VBRig_Error(VBRIG_SHOWLOG_ERROR, VBRigErr, VBRigErrMsg, VBRIG_MODULE_ID_STRING, VBRIG_PROC_ID_STRING)
Call VBRig_Error(VBRIG_POP_PROC_STACK, 0, "", VBRIG_MODULE_ID_STRING, VBRIG_PROC_ID_STRING)
Exit Sub
'+++ VB/Rig End +++
End Sub
|
|
|
|
|
VBRig_Error(VBRIG_POP_HOLY_CRAP, 0, "", VBRIG_MODULE_ID_STRING, VBRIG_PROC_ID_STRING)
Exit Life
Jeroen De Dauw
---
Forums ; Blog ; Wiki
---
70 72 6F 67 72 61 6D 6D 69 6E 67 20 34 20 6C 69 66 65!
|
|
|
|
|
A, Looks like old VB3 code to me.
B, Looks like a quick and dirty throw-away test harness of some sort.
How many times have we seen those turn into production code?
|
|
|
|
|
Here's one that's created in Informix 4GL (which is a horror in itself) on how to validate a from-and-to range to make sure it has no overlaps in the DB:
FUNCTION verify_location()
DECLARE c_det_cur CURSOR FOR
SELECT frerc.from_loc_id, frerc.to_loc_id
FROM frerc
WHERE frerc.dc_id = m_i_frerc_rec.dc_id
AND frerc.whse_id = m_i_frerc_rec.whse_id
# check if select worked correctly
IF SQLCA.SQLCODE <> 0
THEN
#...
RETURN TRUE, 0
END IF
OPEN c_det_cur
IF SQLCA.SQLCODE <> 0
THEN
#...
RETURN TRUE, 0
END IF
FETCH c_det_cur INTO m_from_loc, m_to_loc
IF m_modify = 0 AND m_i_frerc_rec.from_loc_id >= m_from_loc
AND m_i_frerc_rec.from_loc_id <= m_to_loc
THEN
CALL sh_err_msg("S2096")
LET m_from = 1
LET m_modify = 1
END IF
IF m_modify = 0 AND m_i_frerc_rec.to_loc_id >= m_from_loc
AND m_i_frerc_rec.to_loc_id <= m_to_loc
THEN
CALL sh_err_msg("S2097")
LET m_from = 1
LET m_modify = 1
END IF
IF m_from_loc >= m_i_frerc_rec.from_loc_id
AND m_from_loc <= m_i_frerc_rec.to_loc_id
THEN
CALL sh_err_msg("S2097")
LET m_from = 1
LET m_modify = 1
END IF
IF m_modify = 0 AND m_to_loc >= m_i_frerc_rec.from_loc_id
AND m_to_loc <= m_i_frerc_rec.to_loc_id
THEN
CALL sh_err_msg("S2096")
LET m_to = 1
END IF
IF m_to = 0 AND
m_from = 0
THEN
WHILE SQLCA.SQLCODE = 0
FETCH c_det_cur INTO m_from_loc, m_to_loc
IF m_i_frerc_rec.from_loc_id >= m_from_loc
AND m_i_frerc_rec.from_loc_id <= m_to_loc
THEN
CALL sh_err_msg("S2096")
LET m_from = 1
EXIT WHILE
END IF
IF m_i_frerc_rec.to_loc_id >= m_from_loc
AND m_i_frerc_rec.to_loc_id <= m_to_loc
THEN
CALL sh_err_msg("S2097")
LET m_from = 1
EXIT WHILE
END IF
IF m_from_loc >= m_i_frerc_rec.from_loc_id
AND m_from_loc <= m_i_frerc_rec.to_loc_id
THEN
CALL sh_err_msg("S2097")
LET m_to = 1
EXIT WHILE
END IF
IF m_to_loc >= m_i_frerc_rec.from_loc_id
AND m_to_loc <= m_i_frerc_rec.to_loc_id
THEN
CALL sh_err_msg("S2096")
LET m_from = 1
EXIT WHILE
END IF
END WHILE
END IF
LET m_modify = 0
END FUNCTION
|
|
|
|
|
Am I missing something? Why would you do this?
protected int GetTransactionLength(string transaction)
{
return transaction.Length;
}
is this not less code
transaction.Length
than this?
GetTransactionLength(transaction)
This may seem small, but it's just pushed me over the edge. It comes from a program that should be simple and beautiful but it full if this sort of stuff, it’s driving me nutts!
|
|
|
|
|
Depends - was the transaction.Length private? If so, then perhaps someone hasn't got up to properties in the course yet...
No trees were harmed in the sending of this message; however, a significant number of electrons were slightly inconvenienced.
This message is made of fully recyclable Zeros and Ones
|
|
|
|
|
Nope, I have just deleted this method and used the transaction.length and it compiled fine. I could understand it if you want to manipulate the length, so I checked the history of the file in source control to see if it ever did and it's always just returned the length.
|
|
|
|
|
So not only is it silly code, but silly code with public fields? Deep joy...
Is the original coder still there, or have they decided to continue the course instead?
No trees were harmed in the sending of this message; however, a significant number of electrons were slightly inconvenienced.
This message is made of fully recyclable Zeros and Ones
|
|
|
|
|
He left.. He was Microsoft certified, I guess there’s just something’s you can’t teach!
|
|
|
|
|
outside cosmic wrote: He left.. He was Microsoft certified
ftfy
Panic, Chaos, Destruction.
My work here is done.
|
|
|
|
|
In case you want to overload it?
|
|
|
|
|
exactly, i find it very useful to create a Getter/Setter for each member (even if it is public) just in case something changes in the future and it needs to be recovered later.
Example:
if the length now has to adjust, for example, +5 in the current version, simply change the code on GetTranslation(...) { return object.translation+5; } and automatically all the code that refers to this translation is conveniently adjusted.
This is no coding horror, but a wise implementation with future alterability. 
|
|
|
|
|
PLEASE tell me you're joking!?!? 
|
|
|
|
|
This is bizzare and overwrought.
Perhaps he ment to do something like
protected int GetTransactionLength(string transaction)
{
if(transaction == null)
return 0;
return transaction.Length;
}
CCC solved so far: 2 (including a Hard One!)
|
|
|
|
|
And if you get an zero-length string? It's uncommon, but you might want to consider returning -1 instead of zero.
|
|
|
|
|
This wasn't meant to be production level code, just a musing into the weird thought-processes of the original coder of this horror !
0x3c0 wrote: And if you get an zero-length string?
Heavily depends upon the context, but IMO 0 is better as, the need to differentiate between null and "" when getting the length is rare (diffent for the object itself of course).
0x3c0 wrote: It's uncommon
Nope! string foo = ""; or string foo = string.Empty; Are the best ways to declare a string if you don't already have a value for it.
The last time I got into a discussion like this, it really dragged on....
http://www.codeproject.com/Messages/3251798/NULLABLE-type.aspx[^]
CCC solved so far: 2 (including a Hard One!)
|
|
|
|
|
keefb wrote: Heavily depends upon the context, but IMO 0 is better as, the need to differentiate between null and "" when getting the length is rare (diffent for the object itself of course).
You're right; that requirement isn't common. But if I need it, I'd quite like to be able to get it without having to either write another method or altering the original method. It doesn't add much code, and it fits my personal coding style.
Plus, if the string is null, then retrieving Length will cause an exception. I don't like blurring cases together when one of them raises an exception.
|
|
|
|
|
I just made a throw-away observation that perhaps the orginal coder meant to perform some form of null checking and got it wrong (possibly by deleting it), then the error was repeated copied and pasted.
Personally I work hard to eradicate the nulls in the first place. I tend to check parameters for null when they are passed. Becuase I want code where nulls are the driven out, I beleive in handling null reference exceptions (such as the one that would occur on <string>string.length) as exceptions as a null value is an unexpected value (ie indicates a bug). Or at least I attempt this !
That said, if I had to, I'd wrap the <string>string.length as per my method. This has a few of benefits: developers don't have to worry about a special -1 value; No extra code is needed to handle the -1 value (null reference exceptions will, as described above) indicate a true bug ; the result can be displayed in a UI directly as end users would assume the -1 is a bug.
CCC solved so far: 2 (including a Hard One!)
|
|
|
|
|
keefb wrote: Are the best ways to declare a string if you don't already have a value for it.
Of course that depends on the situation, but in general I disagree; I prefer null or uninitialized references.
As a particular situation: What I was working on today involves parsing some user input. For one of the parameters it's OK for the user to not specify a value (a default value will be provided later), but an empty value is not allowed. (It's possible that I'll allow an empty value in the future.) I initialize the variable to null and only assign a value if the user provided a non-empty value.
Another parameter is similar, but an empty value is allowed.
|
|
|
|
|
That's correct, I could not imagine why someone would do it like this.
|
|
|
|
|
Looking for a error in a project, I saw this code.
IFormatProvider culture = new System.Globalization.CultureInfo("en-US", true);
string year = itemDT["YEARBARRIER"].ToString();
string dateYear = "01" + "/" + "01" + "/" + year;
DateTime convertedDate = DateTime.Parse(dateYear, culture, System.Globalization.DateTimeStyles.AssumeLocal);
objBarrierFacade.YearBarrier = convertedDate;
That can be writed with only one line.
objBarrierFacade.YearBarrier = new DateTime((int)itemDT["YEARBARRIER"], 1, 1);
But I think that my solution is so easy and so obviuos to be used and other programmers need a good reason to spend more than a week in some easy use cases.
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
