Trying to understand exceptions in C #

Question

I really don't use any attempts / catches in my code, but I am trying to break this habit and am now getting exceptions.

I believe that the most important place in my application is reading the file, and I'm trying to implement it now, but I'm not sure that you need to use "best practices" for this. I am currently doing something like this:

private void Parse(XDocument xReader) { IEnumerable<XElement> person = xReader.Descendants("Person").Elements(); foreach (XElement e in person) personDic[e.Name.ToString()] = e.Value; if (personDic["Name"] == null || personDic["Job"] == null || personDic["HairColor"] == null) throw new KeyNotFoundException("Person element not found."); } 

But I'm not sure if this is correct. I have this to handle:

 try { personsReader.Read(filename, persons); } catch (KeyNotFoundException e) { MessageBox.Show(e.Message); return; } // Do stuff after reading in the file.. 

However, when e.Message is shown, it just shows a general KeyNotFoundException error message, not a custom error message. In addition, I'm not sure that in general, I’ll talk about this “exception handling” correctly. I return to catch because if the file cannot be read successfully, I just want to pretend that the user never tried to open the file and let it try again with another file.

Am I doing it right? Again, I'm pretty new to using exceptions, and I want to make sure I get it right before continuing and applying it to the rest of my program.

Also, why do people say don't catch (Exception e) ? It seems like in this case I would like to do this because no matter what error occurs when reading in the file, if there is an error, I want to stop reading the file, display an error message and return. Isn't that always the case? I understand that I don’t want to handle Exception e if you want to handle something differently based on an exception, but in this case I would not want to just handle the base class of exceptions in case something goes wrong?

Answer 1

You should catch exceptions when you can handle the condition and do something useful. Otherwise, you should let this bubble up the call stack, and maybe some of you can handle it. Some applications have exception handlers for exception handlers that handle it on the external majority of layers, but, in general, if you don’t know that you have a useful way to handle this, release it.

In your case, you are processing not the ability to read the resource and inform the user. You handle it. Regarding the general exception, one thing you can do is to catch and throw the best exception. If you do, be sure to specify the root cause exception as an internal exception. You can also track or record data if necessary.

 throw new MyGoodExceptionType ("Could not read file", e); // e is caught inner root cause. 

Now the user interface shows a good error and possibly an internal reason in the log, etc.

Some common mistakes:

• Exception handling in the depth of the stack in the shared library method:. Remember that a generic library function can be called in many different codes. You probably have no context on whether to process it and whether to process it. the caller above on the stack probably has a context and knows whether it is safe to handle it. This usually means that higher levels of code will decide to process. In the lower layers, as a rule, you allow them to flow.

• Exclusion of swallowing:. Some codes throw exceptions (especially lower on the stack), and then the root condition just evaporates, which makes it crazy for debugging. Once hagan, if you handle it, do it. If not, let it go.

• Exceptions must be exceptional: Do not use excpetions to control flow. For example, if you are reading a resource, do not try to read, and then catch the exception and make a decision. Instead, call ifexists, check the bool, and make decisions in code. this is especially helpful when you install a debugger to throw exceptions. You should be able to work clean, and if the debugger breaks, this should be a real problem. Constantly interrupting debugging when debugging is problematic. I personally very rarely have to throw exceptions and always try to avoid flow control.

Hope this helps.

Answer 2

Ok, first ...

... This is not a KeynotFoundException, it must be an ArgumentException .... The argument is not valid.

The documentation clearly states:

An exception that is thrown when the key specified to access an item in the collection does not match any key in the collection.

Compare this to:

The exception that is thrown if one of the arguments provided to the method is invalid

Now:

Also, why do people say they don't catch (Exception e)?

Caution, this swallows the exception and makes central error handling / logging impossible. Only expect exception exceptions that you expect IF this is a trick / close of something or its log / retron (i.e. Throw;). Then you have a central appdomain handler that gets every unloaded exceptin and writes it;) It can't handle anything because exceptions at this level are unexpected. It should basically write an extract to a file and be executed, possibly using the user interface (the application has t obe restartet).

Answer 3

As far as you do, it looks mostly normal. I can’t talk about whether you should throw exceptions at this particular moment, but throw and catch correctly. As for the message, it should work as it is. Try displaying e.ToString() to see the call stack. Maybe just making person["Name"] throws a KeyNotFoundException first.

Regarding the issue of catching Exception , this is not always bad. Sometimes you can’t predict all the possible exceptions, and sometimes it’s useful to deal with any possible failure. However, this does not allow you to handle individual exceptions in different ways.

As an example, if you get a KeyNotFoundException , you can mention incorrect file formatting and possibly display the file to the user on the screen. If you get a FileNotfoundException , you can show them the path and open OpenFileDialog so that they select a new file. Security permissions exceptions, you can display instructions to raise your permissions. Some exceptions can be recovered (perhaps one element is poorly formatted, but everything else is in order, if it does not work)?

But, it’s normal to catch everything if you want to design it. The most robust program out there will catch all possible exceptions and handle it in a special way, instead of presenting the original exception to the user. It provides a better user interface and gives you ways to get around problems that might occur.

Answer 4

In most cases, you don’t have to worry about the type of exception that you get, so catching a general Exception fine, however, there are specific situations in which you really would like to catch the corresponding exception (and not just the general Exception ).

One specific example: if you have a thread and you want to interrupt it from a blocking call, in this case you need to distinguish between InterruptException and Exception .

Consider this example: you have a thread that runs Read every minute for 5 minutes (this is not a very realistic example, but it should give you an idea of ​​why you want to handle different exceptions). You have to stop the thread after 5 minutes, because your application will be closed, and you do not want to wait another minute for the running flag to be read ... after all, you do not want your user to wait only a minute to close the application. To stop a thread immediately, you set the flag to false and you call Interrupt on your thread. In this case, you need to catch the ThreadInterrupted exception because it tells you that you should exit the loop. If you catch another exception, then you will not be able to complete the task, but you do not want to give up on work together, and you would like to try and read the next minute again. This shows how your requirements determine the type of exceptions you need to handle. Here is a sample code:

 bool running = true; Thread t = new Thread(()=> { while(running) { try { // Block for 1 minute Thread.Sleep(60*1000); // Perform one read per minute personsReader.Read(filename, persons); } catch (KeyNotFoundException e) { // Perform a specific exception handling when the key is not found // but do not exit the thread since this is not a fatal exception MessageBox.Show(e.Message); } catch(InterruptException) { // Eat the interrupt exception and exit the thread, because the user // has signalled that the thread should be interrupted. return; } catch(Exception e) { // Perform a genetic exception handling when another exception occurs // but do not exit the thread since this is not a fatal error. MessageBox.Show("A generic message exception: " + e.Message); } } }); t.IsBackground = true; t.Start(); // Let the thread run for 5 minutes Thread.Sleep(60*5000); running = false; // Interrupt the thread t.Interrupt(); // Wait for the thread to exit t.Join(); 

Now about your other problem, if the exception is not displayed: note that you are referring to person[e.Name.ToString()] = e.Value , which requires a key search, and if the key is not on the map, you can get a KeyNotFoundException . This will be a general exception that you catch, and your custom exception will never be thrown, because person[e.Name.ToString()] can throw before you even get into your code.

 foreach (XElement e in person) person[e.Name.ToString()] = e.Value; // <-- May be throwing the KeyNotFoundException if (person["Name"] == null || person["Job"] == null || person["HairColor"] == null) throw new KeyNotFoundException("Person element not found."); 

In addition, you do not want to throw a KeyNotFoundException when you really found the key, but you did not find the corresponding value: if person["Name"] == null evaluates to true, then the "Name" key was actually found in the person dictionary, so throwing KeyNotFoundException will mislead anyone who catches this exception. In case your value is null , then it would probably be impractical to throw an exception ... this is really not an exceptional case. You can return a flag indicating that the key was not found:

 public bool PerformRead(/*... parameters ...*/) { foreach (XElement e in person) { // Avoid getting the KeyNotFoundException if(!person.ContainsKey(e.Name.ToString())) { person.Add(e.Name.ToString(), "some default value"); } person[e.Name.ToString()] = e.Value; } if (person["Name"] == null || person["Job"] == null || person["HairColor"] == null) { return false; } else { return true; } } 

Answer 5

I do not quite understand why you are not getting your error message. This should happen (unless something throws a KeyNotFoundException , and not the one you explicitly throw).

In addition, as a rule, you should put all the code that relies on successfully reading the file inside try , which is often the rest of the body of your method. You will no longer need to return inside your catch , and subsequent code that does not rely on the successful reading of the file can still be executed after a failure.

Example:

 public static void Main() { var filename = "whatever"; try { personsReader.Read(filename, persons); var result = personsReader.DoSomethingAfterReading(); result.DoSomethingElse(); } catch (KeyNotFoundException e) { MessageBox.Show(e.Message); } finally { personsReader.CloseIfYouNeedTo(); } DoSomeUnrelatedCodeHere(); } 

And the reason good practice not to catch the old Exception e is because you only want to catch and handle the exceptions that you expect to receive. If you get another exception that you did not expect to receive, this usually means that something new failed as you did not expect, and you want this behavior to be noticeable, and not just cover the mat all regular processing codes mistakes.

Many production level systems will have one big attempt / catch the whole program, which will catch any exception and will log and clean up gracefully. This is complemented by the fact that certain try / catch blocks are deeper inside the code that correctly handle expected exceptions. For unforeseen exceptions, you can always just let the CLR bomb out unceremoniously and find out what came of it.

Here is an example of a new exception. What if something goes horribly wrong on this line:

 IEnumerable<XElement> person = xReader.Descendants("Person").Elements(); 

... do you get an OutOfMemoryException ? If you really just display a popup for the user and let your program try to continue as usual, even if it's just not possible? And what if, because you were silent about an OutOfMemoryException , you later try to dereference a null reference and get a NullReferenceException that causes your program to crash? You will have the devil of time trying to track down the main reason why this link was null.

The best way to hear a mistake is to quickly fail and fail noisily.

Answer 6

“Exceptions for exceptional circumstances” - unknown

Do not use to substantially pass a message from a method to the calling method. Always try to handle things gracefully. When something really strange happens, then throw an exception. This is what dev is not very familiar with how to use exceptions do a lot.

Answer 7

In the code, you call xxx to evaluate the condition inside the if .

Request person["Name"] == null || person["Job"] == null || person["HairColor"] == null person["Name"] == null || person["Job"] == null || person["HairColor"] == null person["Name"] == null || person["Job"] == null || person["HairColor"] == null will fail if any of these keys are not in your dictionary.

You need to do this instead:

 if (!person.ContainsKey("Name"] || !person.ContainsKey("Job"] || !person.ContainsKey("HairColor")) 

So, your call to throw exceptions is never fulfilled! And so you never see your message.

I would keep your habit of not making exceptions for this kind of coding. Exceptions are expensive and can lead to hiding real problems in your code.

Do not allow general exceptions or create exceptions for unforeseen circumstances.