!(How to best use a try catch block) [modified]
-
If the total is expected to always be computable, but it would be better to return -1 than to throw an exception if it isn't, how would you suggest coding it? Testing all four values to see if they're valid, testing to see whether any of the values is so big as to risk overflow, etc. would seem more complicated than just catching an exception. To be sure, such an approach would yield better performance in the case where an exception would be thrown and caught, but worse performance in the expected case where the total can be computed without difficulty. With regard to the relative wisdom of returning -1 versus propagating the exception, that would depend upon what the calling program is going to do with the information. It's not hard to imagine situations where having the function return -1 will be much cleaner than having it throw an exception, and others where the reverse would be true. BTW, one thing I would think would be useful in many situations where some callers would like exceptions handled and others would like them stifled would be for a method to accept a delegate to be called in case of trouble. Such a delegate could throw an exception if necessary, but could also handle some "trouble" situations better than would an exception.
NEVER ever throw an exception to code a foreseeable condition - even if it might be rare. Exceptions are just not made for that, they are for error conditions! It's like knocking in a nail with a wrench: You might succeed somehow, but anyway this is hardly a good and recommendable practice...
supercat9 wrote:
how would you suggest coding it?
public double CrappyTotal // and yes, this 'Total' is an 'Average'
{
get
{
if (this.SomeItem.Details.Length < 4)
{
return -1;
}return (this.SomeItem.Details\[0\].Value + this.SomeItem.Details\[1\].Value + this.SomeItem.Details\[2\].Value + this.SomeItem.Details\[3\].Value) / 4; }}
I really would feel ashamed to code this with an exception instead, it's just ugly and bad craftsmanship... 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. -
Just came across a class which has tens of such properties. These properties are called by other classes and by other properties / methods of this class!!! Oh! and also look how the property name says total, but is actually returning an average :omg:
public double CrappyTotal
{
double CrappyTotal = 0; // notice the name of the variable is the same as the property
try
{
CrappyTotal = this.SomeItem.Details[0].Value;
CrappyTotal += this.SomeItem.Details[1].Value;
CrappyTotal += this.SomeItem.Details[2].Value;
CrappyTotal += this.SomeItem.Details[3].Value;
CrappyTotal = CrappyTotal / 4;
}
catch
{
CrappyTotal = -1;
}
return CrappyTotal;
}modified on Wednesday, July 8, 2009 11:35 PM
-
NEVER ever throw an exception to code a foreseeable condition - even if it might be rare. Exceptions are just not made for that, they are for error conditions! It's like knocking in a nail with a wrench: You might succeed somehow, but anyway this is hardly a good and recommendable practice...
supercat9 wrote:
how would you suggest coding it?
public double CrappyTotal // and yes, this 'Total' is an 'Average'
{
get
{
if (this.SomeItem.Details.Length < 4)
{
return -1;
}return (this.SomeItem.Details\[0\].Value + this.SomeItem.Details\[1\].Value + this.SomeItem.Details\[2\].Value + this.SomeItem.Details\[3\].Value) / 4; }}
I really would feel ashamed to code this with an exception instead, it's just ugly and bad craftsmanship... 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.Hi Thomas, Thanks for that. The day I came across this, I had a private discussion with the developer and reasoned the right way of doing this. The message seems to be slowly getting across :)
-
If I see such a thing i can say only one thing: "Everybody please remain calm! You Sir!! Get your hands up in the air and step away from the keyboard!"
Learn from the mistakes of others, you may not live long enough to make them all yourself.
Lol...good one Unfortunately, I've come across so many such instances of bad programmers in my career (and I'm sure most of you have as well). This fortunately for me has been the only instance where the developer has understood what they've done wrong and positively taken on board the recommendations I've made. I actually came across this while doing a code review for a team that I had not worked with before.
-
NEVER ever throw an exception to code a foreseeable condition - even if it might be rare. Exceptions are just not made for that, they are for error conditions! It's like knocking in a nail with a wrench: You might succeed somehow, but anyway this is hardly a good and recommendable practice...
supercat9 wrote:
how would you suggest coding it?
public double CrappyTotal // and yes, this 'Total' is an 'Average'
{
get
{
if (this.SomeItem.Details.Length < 4)
{
return -1;
}return (this.SomeItem.Details\[0\].Value + this.SomeItem.Details\[1\].Value + this.SomeItem.Details\[2\].Value + this.SomeItem.Details\[3\].Value) / 4; }}
I really would feel ashamed to code this with an exception instead, it's just ugly and bad craftsmanship... 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.Thomas Weller wrote:
NEVER ever throw an exception to code a foreseeable condition - even if it might be rare.
I guess it depends what you mean by 'foreseeable'. Certainly if one could readily determine before performing an operation that it would throw an exception that one was planning on catching, and if there was any significant likelihood of such exception occurring, doing an early test before attempting the operation would be a good idea. Testing the length test on the array, for example, would seem appropriate, though in some applications that may be just about the only exception that could be guaranteed not to occur. On the other hand, it may not be possible to determine in advance all exceptions that might occur. In the code sample given, the '.Value' property could throw an exception (e.g. ObjectDisposed or whatever) even if the length of the array was valid. While a general 'Catch' would be icky, there are times when such a thing is appropriate. For example, in expression-evaluation window, nearly all exceptions thrown during expression evaluation should be caught. An application shouldn't die just because someone typed in an expression that caused an exception the application programmer hadn't foreseen. I don't particularly like -1 as a return value, but the overall style would seem appropriate for some applications.
-
Except that you don't get an error on overflow, you just get PositiveInfinity (or NegativeInfinity), so the only thing that could throw an exception there is the indexing of this.SomeItem.Details You'd just have to test the length/count of that thing, whatever it is..
You'd just have to test the length/count of that thing, whatever it is.. And you have to test all four items to ensure they're not null, and you'd have to somehow know in advance whether any attempt to read .Value might throw an exception of its own (I know properties aren't supposed to throw exceptions, but that doesn't mean they never will).
-
You'd just have to test the length/count of that thing, whatever it is.. And you have to test all four items to ensure they're not null, and you'd have to somehow know in advance whether any attempt to read .Value might throw an exception of its own (I know properties aren't supposed to throw exceptions, but that doesn't mean they never will).
-
Why would there be nulls in there? Well I suppose there could be nulls there, but then I'd still test for null rather than just die and later fix it with a catch..
harold aptroot wrote:
Well I suppose there could be nulls there, but then I'd still test for null rather than just die and later fix it with a catch.
There are all sorts of things that theoretically could go wrong, especially if the object supports inheritance. Checking in advance for all possible exceptions is frequently infeasible at best, and in many situations impossible (since even if the present implementation of a method can't throw an exception, a future override might). If no condition which would cause an exception is expected, such preemptive coding is often a waste of time (both programmer time and execution time). As for catching an exception and returning an error code versus letting the caller handle it, the latter approach is usually better but not always. If one's objective is to store the output of a function when it works, or -1 if it doesn't, which is nicer:
StoreValue(theFunction()) ' Coded to return -1 in case of failure
or
Dim Result as Double
Try
Result = theFunction()
Catch
Result = -1
End Try
StoreValue(Result)I would consider the former approach to be much nicer.
-
Thomas Weller wrote:
NEVER ever throw an exception to code a foreseeable condition - even if it might be rare.
I guess it depends what you mean by 'foreseeable'. Certainly if one could readily determine before performing an operation that it would throw an exception that one was planning on catching, and if there was any significant likelihood of such exception occurring, doing an early test before attempting the operation would be a good idea. Testing the length test on the array, for example, would seem appropriate, though in some applications that may be just about the only exception that could be guaranteed not to occur. On the other hand, it may not be possible to determine in advance all exceptions that might occur. In the code sample given, the '.Value' property could throw an exception (e.g. ObjectDisposed or whatever) even if the length of the array was valid. While a general 'Catch' would be icky, there are times when such a thing is appropriate. For example, in expression-evaluation window, nearly all exceptions thrown during expression evaluation should be caught. An application shouldn't die just because someone typed in an expression that caused an exception the application programmer hadn't foreseen. I don't particularly like -1 as a return value, but the overall style would seem appropriate for some applications.
supercat9 wrote:
On the other hand, it may not be possible to determine in advance all exceptions that might occur.
Of course not. In that case we wouldn't need exceptions at all... What I mean is: 'Foreseeable' in terms of business/application logic. If something totally unexpected happens (like e.g. the 'ObjectDisposed' you mentioned), there should be a general top level exception handler at the application level, but for sure such an exception must never be swallowed silently by the executing code. Not in a million years, in no circumstance whatsoever!
supercat9 wrote:
I don't particularly like -1 as a return value
Same with me, but in the case of my example
-1has a clear, discrete meaning that can de documented (less than 4 items -> computation not possible), while in the original snippet-1means sth. went wrong (and I will never tell you what the problem was...). That's a huge difference - especially on large business-scale software projects, where you might have hundreds of such properties... Regards Thomaswww.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. -
supercat9 wrote:
On the other hand, it may not be possible to determine in advance all exceptions that might occur.
Of course not. In that case we wouldn't need exceptions at all... What I mean is: 'Foreseeable' in terms of business/application logic. If something totally unexpected happens (like e.g. the 'ObjectDisposed' you mentioned), there should be a general top level exception handler at the application level, but for sure such an exception must never be swallowed silently by the executing code. Not in a million years, in no circumstance whatsoever!
supercat9 wrote:
I don't particularly like -1 as a return value
Same with me, but in the case of my example
-1has a clear, discrete meaning that can de documented (less than 4 items -> computation not possible), while in the original snippet-1means sth. went wrong (and I will never tell you what the problem was...). That's a huge difference - especially on large business-scale software projects, where you might have hundreds of such properties... Regards Thomaswww.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.If something totally unexpected happens (like e.g. the 'ObjectDisposed' you mentioned), there should be a general top level exception handler at the application level, but for sure such an exception must never be swallowed silently by the executing code. Not in a million years, in no circumstance whatsoever! That depends what one is trying to do. Would people like Visual Studio very much if it died any time an exception occurred while evaluating expressions in the Watch window? Or is the proper action to show the problematic expression with an indication that it cannot be evaluated? If a program is supposed to graph a function, it may at times be unclear the best way to handle an exception. If one is trying to graph an expression like f(x)=1/x, it's best that the problematic case at x=0 be swallowed, while graphing is allowed to continue on the other side. On the other hand, if the function is one which will always throw an exception, calling the function 1,000 times while drawing the graph will cause 1,000 exceptions to be thrown. Swallowing exceptions in a fashion that hides any indication of what type of exception occurred seems like a bad idea, but it would seem there are many situations where a calling application is merely interested in whether or not an operation worked, and not what went wrong if it didn't.
-
Just came across a class which has tens of such properties. These properties are called by other classes and by other properties / methods of this class!!! Oh! and also look how the property name says total, but is actually returning an average :omg:
public double CrappyTotal
{
double CrappyTotal = 0; // notice the name of the variable is the same as the property
try
{
CrappyTotal = this.SomeItem.Details[0].Value;
CrappyTotal += this.SomeItem.Details[1].Value;
CrappyTotal += this.SomeItem.Details[2].Value;
CrappyTotal += this.SomeItem.Details[3].Value;
CrappyTotal = CrappyTotal / 4;
}
catch
{
CrappyTotal = -1;
}
return CrappyTotal;
}modified on Wednesday, July 8, 2009 11:35 PM
I don't like returning -1 for an error (particularly in this case) because -1 could be a valid result. eg. the values are all -1. There are a couple of nice solutions I can think of. One would be to use a nullable type ie.
double?. The other way would be to let an exception be thrown or throw your own exceptions. Only looking at the first four elements is probably the worst thing IMHO though. What do you think of this?public double CrappyTotal
{
get
{
double total = 0;foreach (var d in this.SomeItem.Details) { total += d.Value; } return total / (double)this.SomeItem.Details.Count; }}
No exception handling :mad:. I would be happy without error handling. I could add a check for nulls at the start, but I don't think it's worth the time. It would probably depend how and where it was being used though. So, are there people out there who insist that every exception must be handled?
-
If something totally unexpected happens (like e.g. the 'ObjectDisposed' you mentioned), there should be a general top level exception handler at the application level, but for sure such an exception must never be swallowed silently by the executing code. Not in a million years, in no circumstance whatsoever! That depends what one is trying to do. Would people like Visual Studio very much if it died any time an exception occurred while evaluating expressions in the Watch window? Or is the proper action to show the problematic expression with an indication that it cannot be evaluated? If a program is supposed to graph a function, it may at times be unclear the best way to handle an exception. If one is trying to graph an expression like f(x)=1/x, it's best that the problematic case at x=0 be swallowed, while graphing is allowed to continue on the other side. On the other hand, if the function is one which will always throw an exception, calling the function 1,000 times while drawing the graph will cause 1,000 exceptions to be thrown. Swallowing exceptions in a fashion that hides any indication of what type of exception occurred seems like a bad idea, but it would seem there are many situations where a calling application is merely interested in whether or not an operation worked, and not what went wrong if it didn't.
supercat9 wrote:
Would people like Visual Studio very much if it died any time an exception occurred while evaluating expressions in the Watch window? Or is the proper action to show the problematic expression with an indication that it cannot be evaluated?
I'm not sure what your argument is here. VS dying under those circumstances isn't "swallowing the exception", it is ignoring it completely. VS reporting the problem isn't swallowing the exception either, it is processing it and reporting the existance of a problem - exactly what exceptions are there for. Swallowing an exception is (IMHO) "pretending it didn't happen" and carrying on regardless. If a problem occurs saving my work every ten minutes and the exception gets swallowed, I am going to be a very unhappy bunny when I find out later it was all lost!
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
-
I don't like returning -1 for an error (particularly in this case) because -1 could be a valid result. eg. the values are all -1. There are a couple of nice solutions I can think of. One would be to use a nullable type ie.
double?. The other way would be to let an exception be thrown or throw your own exceptions. Only looking at the first four elements is probably the worst thing IMHO though. What do you think of this?public double CrappyTotal
{
get
{
double total = 0;foreach (var d in this.SomeItem.Details) { total += d.Value; } return total / (double)this.SomeItem.Details.Count; }}
No exception handling :mad:. I would be happy without error handling. I could add a check for nulls at the start, but I don't think it's worth the time. It would probably depend how and where it was being used though. So, are there people out there who insist that every exception must be handled?
Yes, That is one of the ways I suggested, but also added that if you are calculating an average, then the name should probably reflect that as well :)
-
supercat9 wrote:
Would people like Visual Studio very much if it died any time an exception occurred while evaluating expressions in the Watch window? Or is the proper action to show the problematic expression with an indication that it cannot be evaluated?
I'm not sure what your argument is here. VS dying under those circumstances isn't "swallowing the exception", it is ignoring it completely. VS reporting the problem isn't swallowing the exception either, it is processing it and reporting the existance of a problem - exactly what exceptions are there for. Swallowing an exception is (IMHO) "pretending it didn't happen" and carrying on regardless. If a problem occurs saving my work every ten minutes and the exception gets swallowed, I am going to be a very unhappy bunny when I find out later it was all lost!
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
VS reporting the problem isn't swallowing the exception either, it is processing it and reporting the existance of a problem - exactly what exceptions are there for. The "-1" return seems overly vague as to what problem occurred when computing the data, but it does indicate that a problem occurred. I would not expect the caller of a function like the one given to ignore the return value. It may not process it particularly as an error, but it would probably store it someplace. Suppose the objective is to produce a table showing various data associated with a bunch of people; for whatever reason, certain data will be unavailable for certain people. Exceptions would be appropriate if one's objective was to leave unprocessed any person for whom some information could not be retrieved correctly. Return codes are more appropriate if the objective is to produce a table in which computable values are filled in and uncomputable values show "N/A".
-
Just came across a class which has tens of such properties. These properties are called by other classes and by other properties / methods of this class!!! Oh! and also look how the property name says total, but is actually returning an average :omg:
public double CrappyTotal
{
double CrappyTotal = 0; // notice the name of the variable is the same as the property
try
{
CrappyTotal = this.SomeItem.Details[0].Value;
CrappyTotal += this.SomeItem.Details[1].Value;
CrappyTotal += this.SomeItem.Details[2].Value;
CrappyTotal += this.SomeItem.Details[3].Value;
CrappyTotal = CrappyTotal / 4;
}
catch
{
CrappyTotal = -1;
}
return CrappyTotal;
}modified on Wednesday, July 8, 2009 11:35 PM
