|
It's not only Pokemon exception handling, Smurf naming convention is also widely used in the application I took the snippet from. I just removed it from class name, because it leads in some way to the source of the snippet.
|
|
|
|
|
CDP1802 wrote: if (this == null)
{
throw new NullReferenceException();
}
You don't know how hard we laughed when we found such one in code library.
|
|
|
|
|
Won't ((InspectionMethod*)BadPointer)->Resuscitate() raise the exception ?
|
|
|
|
|
While this is obviously dumb code, in C# and Delphi (and any other language with property support like VB.net), a 'simple assignment' is essentially calling a method so it is as possible to have crash-inducing code in there as it is anywhere else. Particularly if it's something like "myReallyComplicatedRemotingThingy.Connected = true".
|
|
|
|
|
The source code for the property is included. If that causes a crash, you have bigger problems.
Don't forget to rate my post if it helped! 
"He has no enemies, but is intensely disliked by his friends."
"His mother should have thrown him away, and kept the stork."
"There's nothing wrong with you that reincarnation won't cure."
"He loves nature, in spite of what it did to him."
|
|
|
|
|
Yes, obviously in the posted example the developer is just an idiot. Just saying, an assignment sign doesn't automatically make a line of code crash-proof. Even a property getter (which looks totally innocuous) can break stuff, though one can argue that that is bad style in itself (Microsoft do it though, for example trying to read Length out of an unbounded stream).
|
|
|
|
|
too sad you can't override the assignment operator in c#
|
|
|
|
|
I love the error message!
VUnreal wrote: MessageBox.Show(ex.Message, "Serialization error has happened.",
MessageBoxButtons.OK, MessageBoxIcon.Exclamation);
That's assuming an error is thrown. (I've seen the computer do impossible things several times, so I wouldn't put it past it.)
Serialization isn't happening here. No information about where it is happening. Oh shoot, it has ex.Message in it. at first I thought it wouldn't even tell you what was wrong.
Oh, lets throw in an icon to show this is an important error message.
The name of the routine is interesting. You're in a type safe environment that has an invalid type passed into the string and you are going to "fix" it by passing the invalid type back into the string. Really "advanced" thinking going on here.
|
|
|
|
|
Consider a text box with auto complete checking for the existence of the char . or [
Brilliant outsource developer sitting far far away writes the following:
private TextOffsets getTextOffsets(string text, int caretIndex)
{
var heirarchyChars = HeirarchyChars.ToArray();
int previousSeparatorOffset = -1;
if (caretIndex > 0)
{
previousSeparatorOffset = text.LastIndexOfAny(heirarchyChars, caretIndex - 1, caretIndex);
}
...
And of course commits the change... 10 minutes after update contracts kick-in in run time and
ContractException was unhandled
Precondition failed: this == String.Empty || startIndex - count >= 0
Then I go  Update and get this
private TextOffsets getTextOffsets(string text, int caretIndex)
{
var heirarchyChars = HeirarchyChars.ToArray();
int previousSeparatorOffset = -1;
try
{
if (caretIndex > 0)
{
previousSeparatorOffset = text.LastIndexOfAny(heirarchyChars, caretIndex - 1, caretIndex);
}
}
catch (Exception x)
{
}
...
Absolutely brilliant. Let's not fix it.. let's circumvent it... 
By this time curses in Greek are being launched...
Next day update and...
private TextOffsets getTextOffsets(string text, int caretIndex)
{
var heirarchyChars = HeirarchyChars.ToArray();
int previousSeparatorOffset = -1;
int count = caretIndex - 2;
if (caretIndex > 0 && count > 0 )
{
previousSeparatorOffset = text.LastIndexOfAny(heirarchyChars, caretIndex - 1, count);
}
...
 what the...
Pen and paper just to make sure I am not hallucinating... yep we always go back one to check one character forward... Also we have an innovation... one would start an expression for a struct or an array with a . or a [ from position 0 without a variable first. Just brilliant...
Oh well... it should at least be...
private TextOffsets getTextOffsets(string text, int caretIndex)
{
var heirarchyChars = HeirarchyChars.ToArray();
int previousSeparatorOffset = -1;
if (caretIndex > 1)
{
previousSeparatorOffset = text.LastIndexOfAny(heirarchyChars, caretIndex - 1, 1);
}
...
Alberto Bar-Noy
---------------
“The city’s central computer told you? R2D2, you know better than to trust a strange computer!”
(C3PO)
|
|
|
|
|
Nice development environment you have there. Somebody offshore can commit a change that immediately hits production without going through test environment, not bothering with code review, your offshore idiot remains hired... (You are saving $ not paying somebody local, breakdowns of your application reduces costs of running the system for no customers because it doesn't work...)
I'm not too sure about you...
Alberto Bar-Noy wrote: yep we always go back one to check one character forward...
That's for?:
Alberto Bar-Noy wrote: previousSeparatorOffset = text.LastIndexOfAny(heirarchyChars, caretIndex - 1, count);
If caretIndex==10, count would be 8, the start of looking in the array is at 9. If text is a 5 character string, you are allowing the search to start 5 characters past the first valid index location. Same situation, except the string is 10 characters long. You can only look 1 character ahead, yet you are asking for 8, that will blow with the original error you got. (The string has to be 18 characters long or longer for that to work.)
Your statement to look only 1 character forward doesn't make sense either. Going from where you start to the end of the string makes sense to me. Why you are going back 1 character from the index passed doesn't make sense to me either. I'd want to use the last index found to find the next index to be found.
Here's an alternate version:
private TextOffsets getTextOffsets(string text, int caretIndex)
{
var heirarchyChars = HeirarchyChars.ToArray();
var cnt = heirarchyChars.Count();
var strt = caretIndex + 1;
int NewIndex = -1;
if (strt < cnt)
{
NewIndex = text.LastIndexOfAny(heirarchyChars, strt, cnt - strt);
}
Pass in 9 for the starting index for a 10 character string, cnt is 10, strt is 10, doesn't check the string at all.
Of course, since I'm perfect,  I'll go ahead and check this in without even seeing if it will compile...
|
|
|
|
|
well this code never reached production' This is what I was hired for... to kick them out. This code tries to check if the last character typed was . or [
this is an autocomplete for structs and arrays on a PLC Dev. system's only the last char needs to be checked. And in WPF the caret position is after the last character. Finally because of politics (they got their notice so they are still around till Jan. first and I can't fully touch their code.
Alberto Bar-Noy
---------------
“The city’s central computer told you? R2D2, you know better than to trust a strange computer!”
(C3PO)
|
|
|
|
|
Why pass an index field at all? Get the count, subtract 1, use that as the first integer and 1 as your second. If you can't get rid of the field, ignore it and do the same thing.
Don't know enough about WPF, but a string is a string and the caret is either the last character, earlier than the last, or doesn't exist in the string.
|
|
|
|
|
I know! Politics! I can't touch it yet 
Alberto Bar-Noy
---------------
“The city’s central computer told you? R2D2, you know better than to trust a strange computer!”
(C3PO)
|
|
|
|
|
Sorry, it's late, I'm not thinking straight. I can't think of a way to find the length of a string other than text.ToCharArray.Count()
|
|
|
|
|
Int32 contador;
contador = (Int32)0;
and a store procedure with +/- 2600 lines.
|
|
|
|
|
The declaration and initialisation could of course have been done in one line, but that makes little difference. From this I get the impression that the developer who wrote this is very 'modern'. He probably has no idea what the compiler will generate from those simple code lines and added the type cast just in case this assignment might be problematic.
Ironically, assingning zero to a variable of a numeric type is the most unproblematic case of all, since it turns out to be one or more zero bytes, no matter if we are looking at an integer type, a floating point type, signed or unsigned. The compiler knows the size (in bytes) of the variable the value is assigned to and there are no special ways to represent the number 'zero'. Therefore no special type information is needed in the assignment.
Bottom line: I see this as clumsy code, but not as a real horror. It does what it is supposed to and I see no potentially harmful side effects. However, I would also see it as an indicator that the developer should perhaps learn more about what happens under the hood, even if that's not 'modern'.
And from the clouds a mighty voice spoke:
"Smile and be happy, for it could come worse!"
And I smiled and was happy
And it came worse.
|
|
|
|
|
I think, someone may well correct me, that the compiler will ignore the second line as it is repeating the prior step because .net initialises all numbers to zero if no value is explicitly supplied.
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
|
|
|
|
|
No. You would get the 'use of an unassigned variable' error and it would not compile.
Edit: To be precise: I made two assumptions:
1) This variable is declared inside a method, not as a class member. Initializing it separately leaves no other possibility than that.
2) If you declare a variable inside a method and don't initialize it, it's still ok for the compiler as long as you don't try to use it in the following code. Such an unused declaration would only trigger a compiler warning. The error would occur as soon as you tried to use the uninitialized variable (other than initializing it) in the following code.
And from the clouds a mighty voice spoke:
"Smile and be happy, for it could come worse!"
And I smiled and was happy
And it came worse.
modified 22-Nov-11 6:03am.
|
|
|
|
|
Generally, you're right. The exception for that rule is if you declare an int array. It will then initialize every int in the array to 0. Kind of nice that you don't have to go through every row in the array to begin with.
|
|
|
|
|
No that's not true, it only zeroes out the array if you create it, not if you declare it. If you declare it, there is no array, just a variable that could hold it. So there's nothing to zero.
And the array would still have to satisfy the rules of definite assignment. (not its elements of course, which is what you seem to be talking about)
|
|
|
|
|
I was thinking in terms of int[] num = new int[10]; not int[] num;
You're agreeing with me that the 10 ints in the first case will all be zero.
if (num == null) would be true in the latter case because there isn't any value types to compare.
if (num[0] == null) would blow up because the array doesn't exist, I don't think it would compile because a value type can't be null.
I don't think that first "if" would work if you declared int num;
|
|
|
|
|
Uninitialized local variables actually aren't null, but uninitialized. So you can't do if (num == null) if you didn't initialize num, you would get "Error: Use of unassigned variable 'num'".
You actually can compare an int with null, that just gives a warning that it's always false.
|
|
|
|
|
There is a difference between local and declared variables. int IS set to zero here:
bool isEmpty = false;
int[] num;
bool isEmpty2;
int num2;
private void SetEndElement(bool strt, bool outer, bool vert)
{
if (num == null) isEmpty = true;
if (num2 == null) isEmpty2 = true;
The following produces 2 warnings and 2 errors and won't compile
Warning 1 The variable 'isEmpty' is assigned but its value is never used ... (stats on where the error is)
...
Error 3 Use of unassigned local variable 'num' ...
private void SetEndElement(bool strt, bool outer, bool vert)
{
bool isEmpty = false;
int[] num;
bool isEmpty2;
int num2;
if (num == null) isEmpty = true;
if (num2 == null) isEmpty2 = true;
So we're both wrong. 
harold aptroot wrote: warning that it's always false.
|
|
|
|
|
KP Lee wrote: So we're both wrong.
I don't see how you got to this conclusion.
Maybe I have to make it clearer.
- statically uninitialized local variables can not be used.
- comparing an int with null gives a warning that it's always false.
- the thread was about local variables.
- fields in a class are not called "declared variables", but are implicitly set to default(T).
|
|
|
|
|
harold aptroot wrote: - comparing an int with null gives a warning that it's always false.
That statement is where you are wrong. It NEVER says that. At least on my compiler.
With local variables, that results in a fatal error (using uninialized local variables) Absolutely no warning about the comparison. See my prior post for the exact error msg.
If YOUR compiler gives THAT warning, I appologize, I assumed you were using a Microsoft compiler. One new enough to compile (var x = "tst";) I definitely will not vouch for all versions.
harold aptroot wrote: - fields in a class are not called "declared variables", but are implicitly set to default(T).
POE TAE TOE/ PAW TAW TOE
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
