Exception vs Logic
Hello, im going to read a file with File.Readlines().
I read that its better to use logic(like a if-statement) rather then try/catch.
Which of these two would you say is better practice?
Option 1.
try{
fileLines = File.ReadAllLines(fileName);
}catch { throw; }
Option 2.
if (File.Exists(fileName))
{
try
{
fileLines = File.ReadLines(fileName);
}
catch { throw; }
}else { throw new FileNotFoundException(); }
Im thinking option 1 will throw filenotfoundexception either way, and therefore its no reason to check it like in option 2?
Thanks in advance!
47 Replies
catch { throw; } is one of the most useless lines of code you can write
Catching an exception just to throw serves no purpose
Yes, ideally, you should check if the file exists first, then read the file, and not catch the exceptionUnless you're providing the file elsewhere via your method, you might as well use the latter. Only catch an error if you're going to handle it (ie, display a message to the end user)
the less exceptions all around the better.
Also, catching
Exception should be as high as possible, so when you catch exceptions be as specific as possible for as long as possibleOkey thanks you for the input, its a console app and i want to tell the user if the file dosent exist. Is this looking better? 🙂
if (File.Exists(fileName))
{
try{
ileLines = File.ReadLines(fileName);
}
catch(Exception ex) { Console.WriteLine($"Something went wrong when reading file. Message: {ex.Message}. Stacktrace: {ex.StackTrace}"); }
}
else{ Console.WriteLine("File dosent exist!"); }
why do you have the try/catch at all?
My thought is that it might occur an error when reading the file: fileLines = File.ReadLines(fileName);
logging isn't handling the error
so, if it happens it will bubble up to the caller anyway
Okey! So maybe i just check if the file exist, if so -> Read file?
so, catch(Exception ex) is usually bad idea
you want that to be as high as possible
when you catch Exception like that anything that goes wrong ends up in the catch block
For reference (idk if the bot can post here)
put your code in like this
```cs
// code here
```
I really like what mlem once said
Using File.Exist beforehand, just to read the file afterwards is really useless
The trillion amount of factors that could cause not being able to read the file exceed the use of File.Exists,
+ Between the check and the actual reading, the file could be gone - Not really reliable
+ During reading, the file could vanish, now you couldnt read it and the file is gone, though you checked it beforehand
The docs list the exceptions that a method might throw as well as the causes for those exceptions. For example, ReadLines: https://docs.microsoft.com/en-us/dotnet/api/system.io.file.readlines?view=net-6.0
^^
catch those exceptions that are known, if you can handle them
Or even handle them ahead of time. If the user is inputting a 0 length path for example, tell them that they can't
guard statements are good
ArgumentNull exceptions are easily mitigated by using nullable contexts but I wouldn't want you to spin too many plates at once if you're still learning.
That just leads to a TOCTOU bug later
The file could be moved or deleted between the exists check, and the read
And just because it exists doesn't mean you have the permissions to read it
Time-of-check to time-of-use
In software development, time-of-check to time-of-use (TOCTOU, TOCTTOU or TOC/TOU) is a class of software bugs caused by a race condition involving the checking of the state of a part of a system (such as a security credential) and the use of the results of that check.
TOCTOU race conditions are common in Unix between operations on the file sys...
Okey perfect thank you @Naxane! The only way im going to handle the errors is telling the user(writeline).
Atm my code looks like this:
My thought is:
Check if file exist - otherwise tell user.
Surrond content inside if in try/catch so tell user other errors.
Would you rather suggest me to remove the File.Exists() and catch these specific error in a try/catch? 🙂
Yes remove the Exists call
But catch a specific exception type
You almost never want to catch Exception itself
Okey ill try that, thanks you! I have a vague memory of taht timCory(the youtuber) said that you want a try/catch at the lowest level - to be able to trace the error via stacktrace. Am i mistaken?
You catch exceptions where you can reasonably handle them
Okey, and in this case when its a console app that reads a file. The only ways i can think of handling them is either: 1. Tell user(print message/trace) 2. Throw the caught exception(to make app stop running)
Is there any other way you would handle them? 🙂
When folks say handle them, they mean deal with the root cause in a graceful way
Ahh okey!
I thought you meant - do something when exception gets caught
so lets say that you try to read the file, and it's locked, we could potentially handle that with a retry of some sort right?
An input loop that takes the user's query again for example
If you don't have permission to the file, a retry won't really help, one is a temporary condition ( the locked file ), the other requires an external action and can't really be easily handled.
Alternatively if this is a regular windows environment, open a file dialogue
There's also a thing called
The Burden of Knowing, the number of things that might go wrong with reading a file is legendary, the more you know about it, the worse it gets; the more edge cases, what-ifs, you get concerned about goes up, but really mostly it's not a problem, things go wrong, plan for things to go wrong, catch the exceptions that your system is designed to handle and protect your application from everything else.Ive added a division by zero, is this more how the exception handling should be done? (If the zero came from user input maybe restrict the user from not being able to input 0 at the first place) - But just as a example 😉
Forgot the int:
But lets say i catch the error i can handle, for example if i catch the filenotfound exception. If i dont catch the dividedbyzero exception - the program will crash. Im thinking it would be better to catch a general exception(catch(exception ex)) if there are any exception that i have missed?
Or would the better be just to let the program crash? Sorry if im getting it wrong
You will want to have a catch all
But for cases where you have certain behaviour on certain exceptions, you add them
very naive example
Great example, i really understand this topic more now! It is a bit confusing sometimes 🙂
Thanks you so much!
@big OOF this is also pretty important, don't directly output exception data to a user, exception data can leak information about your application pretty easily. Give a succinct but helpful message to the end user, and log the full set of data to a logging system.
File.Create opens a file stream btw
i mean ideally you wouldn't
try-catch at all
try to make sure exceptions don't happen and handle any cases where an exception would happen ahead of time
like you were doing in one of the snippets; check whether the file exists first. if not, don't continue with reading text from it. that way, a FileNotFoundException can't happen in the first placeTime-of-check to time-of-use
In software development, time-of-check to time-of-use (TOCTOU, TOCTTOU or TOC/TOU) is a class of software bugs caused by a race condition involving the checking of the state of a part of a system (such as a security credential) and the use of the results of that check.
TOCTOU race conditions are common in Unix between operations on the file sys...
You have to handle the exception here
An Exists check won't cut it
yeah I was just hand-jamming in some code as a sample
What I just said 
I agree ideally you don't have a try/catch at all, but things can happen, especially when reading a file; there's a lot of unforeseen aspects
What reason is there to not catch-all?
Why would you want to catch
FileNotFound, but not IOException?Or any other of these
it depends on your granularity and what you can handle
there's no reason to catch any exception you can't handle
also, my example was pretty trivial to elaborate the point about catching what you can handle
but a super-catch like catch(Exception) shouldn't be caught at a low level
for something like cosmosdb for instance...
If we catch exception instead we can't get the granularity that our read found nothing, also we don't know what really happened unless we read the error; so if we get some other problem we can't handle why not just let it bubble up?
In that case when you say bubble up, do you mean let the exception get caught by the catch in a lover level, for example main method? 🙂
yes
NB: if you don't have a catch at your entry point then it'll bubble up to the user and vomit out a wall of text
and end your program
whether that's a controller, or a static main, etc.
so your error will travel up the stack until someone catches it and handles it, you don't expressly need a try/catch at every level