Reputation: 22854
In the following chunk of code the new
constructor is documented to throw seven different exception types, including System.IO.PathTooLongException
and System.ArgumentException
, System.UnauthorizedAccessException
and System.SecurityException
:
try
{
var fileInfo = new FileInfo(path);
}
catch ???
I'm just trying to ensure that the path
is actually an accessible path and nothing bad happens when I try to create a file using this path
.
So, the question:
Books and coding guidelines tell me that I shouldn't use catch (Exception)
construct, but I face the following situation - I can catch and handle 4 (out of 7) exception types and their handling for every exception types is absolutely the same.
How should this be done?
I can also think of the following solution, but it still looks bad:
catch (Exception exception)
{
Debug.Assert(exception is PathTooLongException || exception is ...);
// (or maybe rethrow it further if there is a type mismatch)
//... Handling code ...
}
Upvotes: 3
Views: 488
Reputation: 516
I would like to add here, because the Exception handling in almost all java code that I have seen is just incorrect. I.e. you end up with very difficult to debug error for ignored Exceptions, or, equally bad, you get an obscure exception which tells you nothing, because blindly following the "catching(Exception) is bad" and things are just worse.
First, understand that an exception is a way to facilitate the returning of error information across code layers. Now, mistake 1: a layer is not just a stack frame, a layer is code which has a well defined responsibility. If you just coded interfaces and impls just because, well you have better things to fix.
If the layers are well designed and have specific responsibilities, then the information of the error has different meaning as it bubbles up. <-this is the key on what to do, there is no universal rule.
So, this means that when an Exception occurs you have 2 options, but you need to understand where in the layer you are:
A) If you are in the middle of a layer, and you are just an internal, normally private, helper function and something goes bad: DONT WORRY, let the caller receive the exception. Its perfectly OK because you have no business context and 1) You are not ignoring the error and 2) The caller is part of your layer and should have known this can happen, but you might not now the context to handle it down below.
or ...
B) You are the top boundary of the layer, the facade to the internals. Then if you get an exception the default shall be to CATCH ALL and stop any specific exceptions from crossing to the upper layer which will make no sense to the caller, or even worse, you might change and the caller will have a dependency to an implementation detail and both will break.
The strength of an application is the decoupling level between the layers. Here you will stop everything as a general rule and rethrow the error with a generic exception translating the information to a more meaningful error for the upper layer.
RULE: All entry points to a layer shall be protected with CATCH ALL and all errors translated or handled. Now this 'handeled' happens only 1% of the time, mostly you just need (or can) return the error in a correct abstraction.
No I am sure this is very difficult to understand. real Example ->
I have a package that runs some simulations. These simulations are in text scripts. there is a package that compiles these scripts and there is a generic utils package that just reads text files and, of course, the base java RTL. The UML dependency is->
Simulator->Compiler->utilsTextLoader->Java File
1) If something breaks in the utils loader inside one private and I get a FileNotFound, Permissions or what ever, well just let it pass. There is nothing else you can do.
2) At the boundary, in the utilsTextLoader function initially called you will follow the above rule and CATCH_ALL. The compiler does not care on what happen, it just needs to now whether the file was loaded or not. So in the catch, re throw a new exception and translate the FileNotFound or whatever to "Could not read file XXXX".
3) The compiler will now know that the source was not loaded. Thats ALL it needs to know. So if I later I change utilsTestLoader to load from network the compiler will not change. If you let go FileNotFound and later change you will impact compiler for nothing.
4) The cycle repeats: The actual function that called the lower layer for the file will do nothing upon getting the exception. So it lets it go up.
5) As the exception gets to the layer between simulator and compiler the compiler again CATCHES_ALL, hiding any detail and just throws a more specific error: "Could not compile script XXX"
6) Finally repeat the cycle one more time, the simulator function that called the compiler just lets go.
7) The finally boundary is to the user. The user is a LAYER and all applies. The main has a try that catches_ALL and finally just makes a nice dialog box or page and "throws" a translated error to the user.
So the user sees.
Simulator: Fatal error could not start simulator
-Compiler: Could not compile script FOO1
--TextLoader: Could not read file foo1.scp
---trl: FileNotFound
Compare to:
a) Compiler: NullPointer Exception <-common case and a lost night debugging a file name typo
b) Loader: File not found <- Did I mention that loader loads hundreds of scripts ??
or
c) Nothing happens because all was ignored!!!
Of course this assumes that on every rethrow you didn't forget to set the cause exception.
Well my 2cts. This simple rules have saved my life many times...
-Ale
Upvotes: 1
Reputation: 39306
You don't have to catch every possible exception around every line of code.
Instead, only catch when you have context and you are prepared to handle and take appropriate action. Otherwise, just let it bubble up.
Catching exceptions (especially deep in the stack in common functions where you don't have enough context to handle) can be problematic. It sometimes leads to swallowing and not appropriately handling. Handle exceptions when you have context.
For that FileInfo exception it depends how deep in that stack it is. If it's in a library where it can be reached via multiple paths and scenarios, you shouldn't handle, what are you going to do at that point? If that's also the case and it bubble all the way out through a deep stack to a GUI performing an operation where you want to handle it, what are you going to catch? Every possible exception of every code path on it's execution path? And, as code changes, you're going to re-evaluate all paths? In a shallow code path (file info dialog that calls that method) it's an easy call but even then you're not going to handle it differently - you're going to display an error dialog or message in panel. Exception types offer the ability to handle differently. In both of those case, you handle exception so I say bah hum bug to that guideline.
What is very clear is you should not throw type Exception. That short circuits the ability to handle differently.
Another related post:
Trying to understand exceptions in C#
Upvotes: 2
Reputation: 199
Also, you can test the path first with File.Exists(path) or Directory.Exists(path)
Upvotes: 2
Reputation: 720
You could do something like this:
try
{
var fileInfo = new FileInfo(path);
}
catch (System.IO.PathTooLongException ex)
{
handleError(ex);
}
catch (System.ArgumentException ex)
{
handleError(ex);
}
catch (System.UnauthorizedAccessException ex)
{
}
catch (System.SecurityException ex)
{
handleError(ex);
}
public void handleError(Exception ex)
{
//do some stuff
}
Upvotes: 0