Managing Exceptions in .NET



  • For those of you who don't know, Java's exception handling scheme has something called "checked exceptions".  I don't want this thread to turn into a debate about whether checked exceptions are good or not, but suffice it say that Java tells you what exceptions a method might throw.

    .NET doesn't have checked exceptions, so when you call a method there is no built in way to figure out what types of exceptions it could throw.  The .NET framework docs have pretty good descriptions of what exceptions could be thrown and why they are thrown, but for my own code it's tough to match that standard.

    So how does everyone manage exceptions in .NET?  Even if you comment your public methods with the types of exceptions you throw, how do you figure out what exceptions all of the private methods called by the public methods throw?  It seems like fully documenting exceptions would basically involve manually implementing checked exceptions.



  • Any properly documented method explains what it does and how it works. That includes, essentially, the private methods that it calls. Some exceptions you don't need to document, since they're implicit -- things like ArgumentException and IndexOutOfRangeException and DivideByZeroException. Others should probably be explicit. For example, if your method happens to write to a stream (perhaps it writes to a file, perhaps it writes to the network), then you should specify in your documentation that an IOException could be thrown. Or (if your method handled IOException) that it was handled.

    Since you (wisely) don't want to get into a checked exception debate I won't debate it, but I will say this: there are pros and cons to checked exceptions, and there are pros and cons to *not* checking exceptions. C# chose to go the unchecked way.

     If you happen to like checked exceptions, but still want .NET, use J#.



  • If I'm writing a class for public consumption then the public methods will include as part of their documentation what exceptions can be thrown and under what circumstances it might be expected.



  • I try to make sure that all of my methods can't throw an exception.  I use one of two patterns instead:

    1) For a method with a return type of void or a bool that indicates success, I replace the return value with a project wide, standard ProcessResult object.  This object has a bool or enum that indicates success as well as a message and additional information.

    2) For a method that returns another type of object, I have it take as a parameter a delegate with the following prototype: void HandleException(Exception e);.  If the method encounters an exception it calls HandleException which is run in the context of the calling method.

    The downside to these patterns is that my code is littered with boilerplate code for returning ProcessResult objects and try/catch blocks that call HandleException.  The benefits are that these patterns make it easier for me to display meaningful error messages to the user and prevent unhandled exceptions.



  • @rmr said:

    I try to make sure that all of my methods can't throw an exception.  I use one of two patterns instead:

    1) For a method with a return type of void or a bool that indicates success, I replace the return value with a project wide, standard ProcessResult object.  This object has a bool or enum that indicates success as well as a message and additional information.

    2) For a method that returns another type of object, I have it take as a parameter a delegate with the following prototype: void HandleException(Exception e);.  If the method encounters an exception it calls HandleException which is run in the context of the calling method.

    The downside to these patterns is that my code is littered with boilerplate code for returning ProcessResult objects and try/catch blocks that call HandleException.  The benefits are that these patterns make it easier for me to display meaningful error messages to the user and prevent unhandled exceptions.

    wtf? it's not like exception handling is anathema to clean, efficient code. granted, lots of people implement them badly (e.g. flow control) but it sounds like you either don't understand or don't trust the exception model. strange...



  • @Bob Janova said:

    If I'm writing a class for public consumption then the public methods will include as part of their documentation what exceptions can be thrown and under what circumstances it might be expected.

    Okay, but how do you actually figure this out?  It's easy to look at a method and see that it has a Throws in it, but what if it is a 5 line function that calls 5 private helper functions, which in turn may call methods in other objects?  What if the method accepts an interface as a parameter (rather than a class), and calls methods on that interface?  For very high level functions, it's not always obvious what code will even run at runtime, so for me at least it's very hard to figure out what exceptions could be thrown.

    Also, this may just be annoying to me, but documenting exceptions like you suggest can cause big headaches because you basically have to copy and paste sets of exceptions from called methods to calling methods (assuming you aren't catching them).  For example, if you overload a function to have a less specific version that cause one of the other overloads with a default value, you have to copy the exception list from to any of the overloads.  This means that if you decide to add a new possible exception at a very low level of your program, you could end up having to change the documentation of hundreds of methods.

    @sootzoo said:

    @rmr said:
    I try to make sure that all of my methods can't throw an exception.  I use one of two patterns instead:

    1) For a method with a return type of void or a bool that indicates success, I replace the return value with a project wide, standard ProcessResult object.  This object has a bool or enum that indicates success as well as a message and additional information.

    2) For a method that returns another type of object, I have it take as a parameter a delegate with the following prototype: void HandleException(Exception e);.  If the method encounters an exception it calls HandleException which is run in the context of the calling method.

    The downside to these patterns is that my code is littered with boilerplate code for returning ProcessResult objects and try/catch blocks that call HandleException.  The benefits are that these patterns make it easier for me to display meaningful error messages to the user and prevent unhandled exceptions.

    wtf? it's not like exception handling is anathema to clean, efficient code. granted, lots of people implement them badly (e.g. flow control) but it sounds like you either don't understand or don't trust the exception model. strange...


    I think he understands exceptions, he just for whatever reason doesn't want to use them for his error handling model.  This is tough to do though, because .NET only really supports one error handling model (exceptions), and you're not really doing things "The .Net Way" if you use anything else.

    I would say the only positive effect of rmr's scheme is that it will avoid the "unhandled exception" box, but you could do that by overriding .NET's unhandled exception  handler anyway.  The problem doing it rmr's way is that it really is just re-inventing the wheel.  What is the point of giving a method a delegate to handle an exception when you can just use catch?

    I think many people have this idea that exceptions are "bad" and that you should always catch exceptions immediately after they happen.  The cleanest way to use them, though, is to just to let exeptions filter up to the function that actually can handle the exception.  For example, you could catch a FileNotFound exception in low level code, but it's much easier to just let it unwind the stack up to a catch at the UI level (where you can directly ask the user to try a new path).
     



  • Well, for whatever reason, aside from a few UI apps, all of my development has been with asp.net. And, the only thing I've been writing that other people need to consume, lately at least, have all been web services.

    I catch the exceptions I'm prepared to handle at the layer I expect them. Depending on the control flow, I then either throw a domain-specific exception that the next layer up will expect, or just handle the exception in that layer and move on.

    I then throw a generic unhandled exception http handler into the mix to ensure that everything is logged and the user is redirected to a nice 'oops, I blew up' page if something happens that I wasn't expecting. All of my transactions are atomic to the page request lifecycle so I'm not worried about leaving anything in an inconsistant state.



  • @burnmp3s said:

    Okay, but how do you actually figure this out?  It's easy to look at a method and see that it has a Throws in it, but what if it is a 5 line function that calls 5 private helper functions, which in turn may call methods in other objects?  What if the method accepts an interface as a parameter (rather than a class), and calls methods on that interface?  For very high level functions, it's not always obvious what code will even run at runtime, so for me at least it's very hard to figure out what exceptions could be thrown.

    Actually, for many possible exceptions, chances are that you cannot handle them specifically anyway. If there is nothing you can do than log them and/or notify the user, why bother which subtype of exception it is? Concentrate on those few exception where a usefull handling is possible anyway, let all others either run into a catch(Exception x) or don't handle them at all, since a strangely behaving program should better crash than run in an indefinite state.
     



  • @burnmp3s said:

    It seems like fully documenting exceptions would basically involve manually implementing checked exceptions.

     Yeah, well, that's basically what I do... Kinda a PITA, but oh well... I know my personal projects aren't shining examples of perfect code. I try not to deal with exceptions unless I can see that there's a possibility of one being thrown. It's just not hard to keep track of it yourself I find.
     



  • @sootzoo said:

    wtf? it's not like exception handling is anathema to clean, efficient code. granted, lots of people implement them badly (e.g. flow control) but it sounds like you either don't understand or don't trust the exception model. strange...

    I suppose I'm willing to believe that there is something I don't understand about the exception model, care to help me out?  What do you feel is the proper way to use them?

    Personally, I would much rather deal with code that has exactly one exit than one with a bunch of comments telling me all the ways it could error out.



  • I wasn't taking a swipe so much as questioning your rationale. You have a static method to handle exceptions "in the context of the calling method". Doesn't "catch" suit your purposes? Need it to bubble up further, you can always throw afterward or just catch a more generic exception type further up the chain. Unifying every exception handler throughout the app seems like you'd lose the benefit of context, but maybe I'm not understanding how your implementation works, here.

    The ProcessResult object actually seems like a somewhat elegant solution to otherwise kludgy ways of handling multiple return values like extra reference parameters or object arrays, so I'm genuinely curious why you're doing what you do with exception handling. It seems... off, to me.


  • Wow. I must say that, as a heavy "java" user, I'm quite shocked by the state of things. Not having used .NET myself yet, I still had a very good opinion of the C# language and supporting libraries, with plenty of useful language additions. This, and the resulting 'solutions' are the first time I really have to wonder what they were thinking when designing the language.

    Checked exceptions seem like such a basic feature and are such a natural way of handling errors that I'd really hate to give them up. 



  • @rmr said:



    I suppose I'm willing to believe that there is something I don't understand about the exception model, care to help me out?  What do you feel is the proper way to use them?
    Personally, I would much rather deal with code that has exactly one exit than one with a bunch of comments telling me all the ways it could error out.


    You don't want to deal with code that has exactly one exit. Suppose you have a function with multiple points of failure: open a socket, read from a file, write to a socket.. How would you code that?

    The most sensible approach is to return as soon as possible:

    socket s = open_socket(..);
    if( socket == 0)
        return -E_SOCKET;
    file f = open_file(...);
    if( file == 0)
        return -E_FILE;

    while( !eof )
    {
        bytes = read(file, buffer);
        if( bytes = 0 )
        {
           ret = write(socket, buffer);
            if( ret == -1)
                return -E_CONNCLOSED;
        }
    }

    If you try to reduce that to a single point of exit then you'll have to drag LOTS of variables all the way through the function.

    socket s = open_socket(..);
    if( socket == 0)
        error = 1;

    if( !errror)
    {
        file f = open_file(...);
        if( file == 0)
            error = 1;

        while( !error && !eof )
        {
            bytes = read(file, buffer);
            if( bytes = 0 )
            {
               ret = write(socket, buffer);
                if( ret == -1)
            {
                    error = 1;
                break;
            }
            }
        }
    }

    return error > 1 ? error : retval;

    It results in lots of status variables that tell the remaining code not to execute. It's utterly pointless, hard to read, and should be eliminated. A function should return _AS SOON AS POSSIBLE_ (Also see http://www.codinghorror.com/blog/archives/000486.html or the Refactoring book by Fowler et al.)

    Now, once you've accepted the idea of multiple return points benefiting your health and sanity, you may even like the idea of exceptions, as they greatly simplify the first piece of code.

    Instead of using magically defined E_ERROR values, you use a class name instead. "return -E_FILE;" becomes "throw new FileNotFoundException();" and the magic value disappears. Exceptions are a way of telling the compiler about all the 'magic error values' that you use to indicate problems. The compiler can then make sure that it's all accounted for.

    It's very convenient as well. Suppose that a read_foo() function you're using encounters an error internally and returns -E_FOONOTFOUND; Now your function checks the return and returns -E_MYERROR; The caller of your function checks that and again returns the same or another error constant. That's lots of redundant code that the compiler can easily take care of you. Just declare that the error can occur ("throws FooException") and it will be passed up to calling function until something can handle the error.

    In summary, exceptions are fool proof error constants with lots of added benefits.They're safer and easier to use than magic numbers. 



  • @Nandurius said:

    Wow. I must say that, as a heavy "java" user, I'm quite shocked by the state of things. Not having used .NET myself yet, I still had a very good opinion of the C# language and supporting libraries, with plenty of useful language additions. This, and the resulting 'solutions' are the first time I really have to wonder what they were thinking when designing the language.

    Checked exceptions seem like such a basic feature and are such a natural way of handling errors that I'd really hate to give them up. 

    No offense, but if you're shocked that another programming language doesn't have checked exceptions, then you probably haven't tried using very many other languages.  All new programming languages have had some form of exceptions for years now, but Java was and still is the only major language that uses checked exceptions (CLU language used them, but it's a fairly obscure language for the 70s).  It's probably the most controversial aspect of the Java language.  The .NET designers considered adding checked exceptions, but decided against it (try searching google for "C# checked exceptions").

    Also, although it doesn't use checked exceptions, you might want to check out the Eiffel language.  It uses a system called "Design By Contract" to add certain requirements (in the form of assertions) to methods.  This is similar to checked exceptions in that you are adding more design time information about each function in order to prevent runtime errors.  I beleive there are also extentions for Java to add Design By Contract to the language.



  • @burnmp3s said:

    No offense, but if you're shocked that another programming language doesn't have checked exceptions, then you probably haven't tried using very many other languages.  All new programming languages have had some form of exceptions for years now, but Java was and still is the only major language that uses checked exceptions (CLU language used them, but it's a fairly obscure language for the 70s).  It's probably the most controversial aspect of the Java language.  The .NET designers considered adding checked exceptions, but decided against it (try searching google for "C# checked exceptions").

    Also, although it doesn't use checked exceptions, you might want to check out the Eiffel language.  It uses a system called "Design By Contract" to add certain requirements (in the form of assertions) to methods.  This is similar to checked exceptions in that you are adding more design time information about each function in order to prevent runtime errors.  I beleive there are also extentions for Java to add Design By Contract to the language.

    I've used plenty of languages that do not have exceptions, it's just that the .NET framework is so similar to Java that I find it hard to believe they missed that last step :-) It seems like  such a trivial thing to check, at compile time anyway, I'm surprised there's not even a compiler option to turn it on.

    It's true though that I've had the "fortune" of using java for most projects so far. Maybe I just don't realize the full horror of the other programming environments out there yet :)



  • @burnmp3s said:

    @Nandurius said:

    Wow. I must say that, as a heavy "java" user, I'm quite shocked by the state of things. Not having used .NET myself yet, I still had a very good opinion of the C# language and supporting libraries, with plenty of useful language additions. This, and the resulting 'solutions' are the first time I really have to wonder what they were thinking when designing the language.

    Checked exceptions seem like such a basic feature and are such a natural way of handling errors that I'd really hate to give them up. 

    No offense, but if you're shocked that another programming language doesn't have checked exceptions, then you probably haven't tried using very many other languages.  All new programming languages have had some form of exceptions for years now, but Java was and still is the only major language that uses checked exceptions

    It depends on how you define "major language". If you just mean "Java, C++, and C" then it's no real surprise that Java's the only one with checked exceptions.

    Haskell and ocaml both have checked exceptions (ocaml has a separate exception-checking tool, so it's optional; Haskell integrates exception checking into its type system via monads, so there's no extra system or syntax for exceptions).



  • @Nandurius said:

    <explanation> 

    In summary, exceptions are fool proof error constants with lots of added benefits.They're safer and easier to use than magic numbers. 

    Sounds good to me, I'm willing to give it a try.  My only issue is that you are making the calling code more complicated, since it now needs to know all the possible exceptions I can throw.  You end up with a whole bunch of catches if you want to provide a helpful error message.  Personally, I think I'd rather have the caller be simple and the callee be complicated than the other way around.  Like I said though, I'll give it a try.

    I got out my copy of Refactoring and checked it out.  My code is actually very close to his example for checked exceptions (p 313 in my copy) except that I typically use a delegate to handle the exception.  Since the delegate is usually run in the context of the UI, it makes it easy to provide a meaningful error message to the user.



  • @rmr said:

    @Nandurius said:

    <explanation> 

    In summary, exceptions are fool proof error constants with lots of added benefits.They're safer and easier to use than magic numbers. 

    Sounds good to me, I'm willing to give it a try.  My only issue is that you are making the calling code more complicated, since it now needs to know all the possible exceptions I can throw.  You end up with a whole bunch of catches if you want to provide a helpful error message.  Personally, I think I'd rather have the caller be simple and the callee be complicated than the other way around.  Like I said though, I'll give it a try.

    I got out my copy of Refactoring and checked it out.  My code is actually very close to his example for checked exceptions (p 313 in my copy) except that I typically use a delegate to handle the exception.  Since the delegate is usually run in the context of the UI, it makes it easy to provide a meaningful error message to the user.

     

    Can you give me a trivial example of where the caller would need to know the exception thrown by a callee?

     Can't the caller just know that an exception was thrown and display what it was and exit gracefully? (as ammoQ said, i think, you don't want a program to continue running in an indefinite state)

    I'm of the class of programmers that thinks that if your code is probably going to throw exceptions like that, you're doing something wrong (I.E. Trying to sync to a database that doesn't exist, etc)

    in which case you need to exit.

    everytime i've ever used a try .. catch ... end try block it's been when i want the program to close if it throws. if my program is throwing for another reason, i usually debug the code until it can't throw for any other reason except the ones that i catch explicitly. This of course doesn't include out of memory errors and "jesus you hit me with a bat" errors or the like, but why should *I* have to catch those, it's the framework's job, or the operating system's.



  • @Nandurius said:

    @burnmp3s said:
    No offense, but if you're shocked that another programming language doesn't have checked exceptions, then you probably haven't tried using very many other languages.  All new programming languages have had some form of exceptions for years now, but Java was and still is the only major language that uses checked exceptions (CLU language used them, but it's a fairly obscure language for the 70s).  It's probably the most controversial aspect of the Java language.  The .NET designers considered adding checked exceptions, but decided against it (try searching google for "C# checked exceptions").

    Also, although it doesn't use checked exceptions, you might want to check out the Eiffel language.  It uses a system called "Design By Contract" to add certain requirements (in the form of assertions) to methods.  This is similar to checked exceptions in that you are adding more design time information about each function in order to prevent runtime errors.  I beleive there are also extentions for Java to add Design By Contract to the language.

    I've used plenty of languages that do not have exceptions, it's just that the .NET framework is so similar to Java that I find it hard to believe they missed that last step :-) It seems like  such a trivial thing to check, at compile time anyway, I'm surprised there's not even a compiler option to turn it on.

    It's true though that I've had the "fortune" of using java for most projects so far. Maybe I just don't realize the full horror of the other programming environments out there yet :)

    I don't think the original author wanted this to devolve into a checked vs unchecked exceptions flame war, so I will attempt to cut things off at the pass by saying this:

    Go do as burn suggested; read up on *why* C# decided against checked exceptions. Here's a link to get you started:

    It's a "chat" with the main architect of C# Anders Hejlsberg, and it centers around checked exceptions and the decision to leave them out of C#. I'll give you a super-short summary:

    Checked exceptions are essentially a good idea, but as implemented in Java they are broken in that they tend to encourage bad programming behavior, break versionality, and do not scale. Examples of bad behavior include adding throws Exception clauses to every method (completely defeating the purpose) or using empty catch blocks (again, defeating the purpose) -- I have seen empty catch blocks encouraged in most Java books and online tutorials that I've read, so he has a valid point there. Until a "good" version of checked exceptions can be conceived, C# will remain silent on the issue.



  • @rmr said:

    Sounds good to me, I'm willing to give it a try.  My only issue is that you are making the calling code more complicated, since it now needs to know all the possible exceptions I can throw.  You end up with a whole bunch of catches if you want to provide a helpful error message.  Personally, I think I'd rather have the caller be simple and the callee be complicated than the other way around.  Like I said though, I'll give it a try.

    I don't think that the calling code gets that much more complicated. Most functions only throw one or maybe two different exceptions.

    - If you can handle them, you'd need an if() statement checking the return value anyway, so a catch is not more complex.
    - If you can't handle them, declare the function to throw that type of exception and don't write any additional code (this actually saves you the return value check and return statement.)

    - If you let a lot of exceptions filter up that way, you will, eventually, of course have to write a whole lot of catch statements. If you don't care about the error and just want to show it to the user and abort, then you'll unfortunately end up with a lot of redundant catch clauses. One way to avoid this is if the exceptions have a common superclass. That way, you only need one catch statement that will also work on all subclasses. Unfortunately the Java libraries have extended RuntimeExceptions (unchecked) from Exception, along with all checked exceptions. That means that

    try
    {
       do_something_big(); // can throw 10 different exceptions
    }
    catch (Exception e)
    {
       System.out.println("Oops: " + e);
    }

     will unfortunately ALSO catch all RuntimeExceptions. So doing that would mask things like OutOfMemoryExceptions, which probably isn't what you want to happen. You can work around that though, by catching RuntimeExceptions first and rethrowing them, and then swallow any checked exception, if you really don't care what happened.
     

    try
    {
       do_something_big(); // can throw 10 different exceptions
    }
    catch (RuntimeException e)
    {
       throw e;
    }
    catch (Exception e)
    {
       System.out.println("Oops: " + e);
    }
    Not really idea, I agree, but better than copy&pasting the same handling code into 10 different catch blocks :) 


  • @sootzoo said:

    I wasn't taking a swipe so much as questioning your rationale. You have a static method to handle exceptions "in the context of the calling method". Doesn't "catch" suit your purposes? Need it to bubble up further, you can always throw afterward or just catch a more generic exception type further up the chain. Unifying every exception handler throughout the app seems like you'd lose the benefit of context, but maybe I'm not understanding how your implementation works, here.

    The ProcessResult object actually seems like a somewhat elegant solution to otherwise kludgy ways of handling multiple return values like extra reference parameters or object arrays, so I'm genuinely curious why you're doing what you do with exception handling. It seems... off, to me.

    Not to be pedantic but passing a delegate is in no way the same thing as static method.  I use the delegate to pass a closure representing the state of the calling method to the callee.  When the delegate runs, it does so, as I said, in the context of the calling code, meaning that it can access all objects and methods that are available to the caller.  Please note that this is literally exactly the same as throwing the exception up to the caller, except in my case all of the exception handling code is placed in a separate method from the calling method.

    That's fine if my (very slightly different) way of handling exceptional cases seems "off" to you.  Exception handling is very nearly a religious debate among programmers, and I don't intend to continue the debate much longer.  Once again, I'm perfectly willing to try your way of doing things.



  • One possible .NET method for Nandurius's approach would be to use ApplicationException or extend ApplicationException for 'checked' exceptions. Then, if you caught an ApplicationException, you'd know your app wasn't happy with something. If you caught an Exception, you'd know your runtime wasn't happy with something.



  • rmr, can you post a quick sample? I'm honestly interested to see it in action... it sounds pretty cool in action.



  • @GeneWitch said:

    Can you give me a trivial example of where the caller would need to know the exception thrown by a callee?

    I don't know if you were talking to me or not, but yeah, typically I think you're 100% correct.  The caller doesn't need to know the specifics of what exception was thrown, it simply needs to know that something bad happened.  However if something bad does happen, you want it to fail gracefully.  Especially with web apps, you don't ever want to show the exception text to the user.  Most of the time you just want to tell them that whatever they were doing didn't go so well and you want to write an error message to an event log.
     



  • @GeneWitch said:

    Can you give me a trivial example of where the caller would need to know the exception thrown by a callee?

     Can't the caller just know that an exception was thrown and display
    what it was and exit gracefully? (as ammoQ said, i think, you don't
    want a program to continue running in an indefinite state)

    The classic example is file handling, e.g. saving a file.

    Calling fileSave(filename), the UI could really do with the specifics of the exception thrown, rather than just "there was an error":


    DriveNotPresentException - "you moron, put the floppy back in the drive"

    AccessViolationException - "you don't have permission to access this file"

    FileReadOnlyException - "file is marked as read-only so can't be written to"

    FileSystemReadOnlyException - "duh, try not saving to read-only partition"

    etc... etc... 

     

    In most other cases, errors can fall into two or more basic categories - permanent or temporary errors. It's worth knowing if it's possible/sensible to try again or not. Is it an exception probably caused by bad info from the user that can be corrected, or is it a critical system failure? You want to handle these things differently, so knowing the type of exception seems very important to me.



  • @RayS said:

    @GeneWitch said:

    Can you give me a trivial example of where the caller would need to know the exception thrown by a callee?

     Can't the caller just know that an exception was thrown and display
    what it was and exit gracefully? (as ammoQ said, i think, you don't
    want a program to continue running in an indefinite state)

    It's worth knowing if it's possible/sensible to try again or not. Is it an exception probably caused by bad info from the user that can be corrected, or is it a critical system failure? You want to handle these things differently, so knowing the type of exception seems very important to me.

     It really depends on how you decide to use exceptions in your code.  If you throw a lot of exceptions in your code, you will end up using them for more than just fatal application errors.  I tend to use exceptions more heavily than most people.  Here are my general rules of thumb:

     

    • I throw exceptions instead of returning error values or nulls.  For example, if I was writing a phonebook class, I would throw an exception if someone called entry = phonebook.Lookup("Jimmy") and "Jimmy" was not in the phonebook.
    • I always make sure there is a way to avoid an exception.  In the phonebook example, I would have a phonebook.IsListed("Jimmy") function.
    • I throw exceptions if something goes wrong in the middle of a complicated process.  For example, if phonebook.LoadFromFile() can't find the file, or has trouble parsing the file, I will throw an exception.
    • I throw exceptions if something goes wrong in hardware or network resources.  For example, phonebook.ReadFromInternet might throw some sort of communication exception if the user is not connected to the internet.
    • I never throw the "Exception" base class, I always throw the most specific exception that will make sense to the calling code.
    • I use try...finally (with no catch) or "using" in places where I need to clean up resources.  I always assume some sort of exception could be thrown at any time.
    • I don't have very many catch blocks.  Sometimes I will catch an exception, set it as an inner exception of my own custom exception, and then throw the custom exception.  Most of the exceptions filter up to the UI where they are handled (ask the user to try again).

     



  • @burnmp3s said:

    • I throw exceptions instead of returning error values or nulls.  For example, if I was writing a phonebook class, I would throw an exception if someone called entry = phonebook.Lookup("Jimmy") and "Jimmy" was not in the phonebook.

    IMO a bad idea. java.util.Map doesn't do that, System.Collections.IDictionary doesn't do that, so why should you? This is a suprise for other programmers.

     

    • I always make sure there is a way to avoid an exception.  In the phonebook example, I would have a phonebook.IsListed("Jimmy") function.

    This leads to bloated code and degrades performance. I don't care that much about performance, but I care a lot about bloated code. In some cases, it may also lead to race condition bugs.




  • @ammoQ said:

    @burnmp3s said:
    • I throw exceptions instead of returning error values or nulls.  For example, if I was writing a phonebook class, I would throw an exception if someone called entry = phonebook.Lookup("Jimmy") and "Jimmy" was not in the phonebook.

    IMO a bad idea. java.util.Map doesn't do that, System.Collections.IDictionary doesn't do that, so why should you? This is a suprise for other programmers.

    Check the docs for Generics.Dictionary.Item in .NET (http://msdn2.microsoft.com/en-us/9tee9ht2.aspx), it throws a "KeyNotFoundException" if you try to lookup a missing key.  You are right though, returning a null value in this situation is very common.  Anyway, like I said, I tend to throw exceptions more than most people do.

    @ammoQ said:

    @burnmp3s said:
     

    • I always make sure there is a way to avoid an exception.  In the phonebook example, I would have a phonebook.IsListed("Jimmy") function.

    This leads to bloated code and degrades performance. I don't care that much about performance, but I care a lot about bloated code. In some cases, it may also lead to race condition bugs.

    I don't think these types of methods lead to bloated code.  Could you explain your reasoning?  You have to do the check anyway, its just a question of whether you want to check IsListed and then Lookup, or Lookup and check for null.  Note that using an exception has the added benefit that you can let it propagate up to the UI where you can ask the user to try again, whereas if you check for null you always have to write code to do something about it (to avoid a null reference exception).  In general I write less code when I use exceptions than when I use error values (like null).

    You're right about the race condition, if you are in a threaded situation where a listing could be removed, you could get back true from IsListed but hit a NotFound exception in Lookup.  In this example the safest thing to do to make it thread safe might be to lock the phonebook (with a Mutex or Monitor), do a series of operations on it, and then unlock it.  For example, you could have phonebook.Open() and phonebook.Close()/Dispose() that would do the locking, the same way that opening a port or file locks it until you close it.

    Anyway I'm not saying this is the best way to use exceptions, or that everyone should use exceptions the way I do, I'm just saying its one way to use exceptions.  There are pros and cons to it, like there is with anything.  All I know is that before I started doing things this way, I would get null reference exceptions once in a while that I would eventually track down to a place where I didn't check a value for null.  Now that I throw exceptions instead of returning nulls, when I make a similar mistake I get a nice exception that tells me exactly where the bug is.



  • @burnmp3s said:

    @ammoQ said:
    @burnmp3s said:
    • I throw exceptions instead of returning error values or nulls.  For example, if I was writing a phonebook class, I would throw an exception if someone called entry = phonebook.Lookup("Jimmy") and "Jimmy" was not in the phonebook.

    IMO a bad idea. java.util.Map doesn't do that, System.Collections.IDictionary doesn't do that, so why should you? This is a suprise for other programmers.

    Check the docs for Generics.Dictionary.Item in .NET (http://msdn2.microsoft.com/en-us/9tee9ht2.aspx), it throws a "KeyNotFoundException" if you try to lookup a missing key.  You are right though, returning a null value in this situation is very common.  Anyway, like I said, I tend to throw exceptions more than most people do.

    A wide variety of .Net Collections throw exceptions when the keys are not found including Hashtable, System.DirectoryServices.PropertyCollection, and System.Collections.Specialized.NameValueCollection.  At least in the .Net world, this behavior is common. 

    @burnmp3s said:

    @ammoQ said:
    @burnmp3s said:
     

    • I always make sure there is a way to avoid an exception.  In the phonebook example, I would have a phonebook.IsListed("Jimmy") function.

    This leads to bloated code and degrades performance. I don't care that much about performance, but I care a lot about bloated code. In some cases, it may also lead to race condition bugs.

    I don't think these types of methods lead to bloated code.  Could you explain your reasoning?  You have to do the check anyway, its just a question of whether you want to check IsListed and then Lookup, or Lookup and check for null.  Note that using an exception has the added benefit that you can let it propagate up to the UI where you can ask the user to try again, whereas if you check for null you always have to write code to do something about it (to avoid a null reference exception).  In general I write less code when I use exceptions than when I use error values (like null).

    ... snip ...

    I also disagree that this leads to bloated code but I'm coming from the .Net world view where most key/value collections already have a Contains or ContainsKey method.  I see the IsListed method as pretty much the same thing.  I'll grant you that in a multi threaded environment it might present a opportunity for race condition bugs. 



  • @burnmp3s said:

    I don't think these types of methods lead to bloated code.  Could you explain your reasoning?  You have to do the check anyway, its just a question of whether you want to check IsListed and then Lookup, or Lookup and check for null.  Note that using an exception has the added benefit that you can let it propagate up to the UI where you can ask the user to try again, whereas if you check for null you always have to write code to do something about it (to avoid a null reference exception).  In general I write less code when I use exceptions than when I use error values (like null).

    My thoughts on this is that on some data structures, i.e. linked lists, you may have to iterate over the entire list to find out whether an item is contained in the collection or not. If it is, you'll have to iterate over possibly the entire collection again. That's not particularly efficient. The null check, in comparison, is a two instruction penalty, no matter how big the collection.



  • @burnmp3s said:

    I don't think these types of methods lead to bloated code.  Could you explain your reasoning?

    In some situations, you need the check anyway, so it doesn't matter; in other situations, the return value of null is perfectly what you need.

    I also do not like the idea that you have to write the expression for the key twice...

    if (Phonebook.IsListed(whatever.Param["name"+foobar.Id])) {
      entry = Phonebook.Lookup(whatever.Param["name"+foobar.Id]);
      ...
    }

    To avoid repeating yourself, you have to put the key into a variable, thus adding one more line... Compare that to

    if ((entry = Phonebook.Lookup(whatever.Param["name"+foobar.Id])) != null) {
      ...
    }

     

    @lpope187 said:

    A wide variety of .Net Collections throw exceptions when the keys are not found including Hashtable, System.DirectoryServices.PropertyCollection, and System.Collections.Specialized.NameValueCollection.  At least in the .Net world, this behavior is common. 

    Unless the online doc at MSDN is wrong, System.Collection.Hashtable returns null if the key is not found.

    System.Collections.Specialized.NameValueCollection seems to return null, too...

    Found here


    The Get method does not distinguish between a null reference (Nothing in Visual Basic) which is returned because the specified key is not found and a null reference (Nothing in Visual Basic) which is returned because the value associated with the key is a null reference (Nothing in Visual Basic).

    I guess either you are wrong, or the guys at MS don't know their own framework.



  • @ammoQ said:

    @lpope187 said:


    A wide variety of .Net Collections throw
    exceptions when the keys are not found including Hashtable,
    System.DirectoryServices.PropertyCollection, and
    System.Collections.Specialized.NameValueCollection.  At least in the
    .Net world, this behavior is common. 

    Unless the online doc at MSDN is wrong, System.Collection.Hashtable returns null if the key is not found.

    System.Collections.Specialized.NameValueCollection seems to return null, too...

    Found here


    The Get method does not distinguish between a null reference (Nothing in Visual Basic) which is returned because the specified key is not found and a null reference (Nothing in Visual Basic) which is returned because the value associated with the key is a null reference (Nothing in Visual Basic).

    I guess either you are wrong, or the guys at MS don't know their own framework.

    I stand corrected - maybe next time I'll search the documentation before I open my big mouth. 

     



  • @ammoQ said:

    I also do not like the idea that you have to write the expression for the key twice...

    if (Phonebook.IsListed(whatever.Param["name"+foobar.Id])) {
      entry = Phonebook.Lookup(whatever.Param["name"+foobar.Id]);
      ...
    }

    To avoid repeating yourself, you have to put the key into a variable, thus adding one more line... Compare that to

    if ((entry = Phonebook.Lookup(whatever.Param["name"+foobar.Id])) != null) {
      ...
    }

    I know this is completely a matter of personal preference,  but I tend to stay away from doing so many things on one line.  I try to make my intentions clear by seperating each step into a different line.  I completely understand the other point of view, but I like when I can just glance at the if expression and immediately know what it's doing.

    I tend to only put booleans in if statements, rather than expressions.  For example:

    if (((number % 2) != 0) && (number > 0) && (number < max))

    You have to read all of the expressions to figure out what this is statement is checking for (and these aren't even very complicated expressions).  I would rather do this:

    bool isOdd = ((number % 2) != 0);
    bool isPositive = number > 0;
    bool isLessThanMax = number < max;
    if (isOdd && isPositive && isLessThanMax)

    Giving each of the subexpressions names helps me give more information to the person reading the code.  This makes the code longer, but for me it's a tradeoff that's worth it.
     



  • @burnmp3s said:

    @ammoQ said:

    I also do not like the idea that you have to write the expression for the key twice...

    if (Phonebook.IsListed(whatever.Param["name"+foobar.Id])) {
      entry = Phonebook.Lookup(whatever.Param["name"+foobar.Id]);
      ...
    }

    To avoid repeating yourself, you have to put the key into a variable, thus adding one more line... Compare that to

    if ((entry = Phonebook.Lookup(whatever.Param["name"+foobar.Id])) != null) {
      ...
    }

    I know this is completely a matter of personal preference,  but I tend to stay away from doing so many things on one line.  I try to make my intentions clear by seperating each step into a different line.  I completely understand the other point of view, but I like when I can just glance at the if expression and immediately know what it's doing.

    I can aggree to that; my example is, after careful consideration, probably not a good one, especially since = in an if statement easily leads to misinterpretations. Let's make it

    entry = Phonebook.Lookup(whatever.Param["name"+foobar.Id]);
    if (entry
    != null) {
      ...
    }



  • @burnmp3s said:

    if (((number % 2) != 0) && (number > 0) && (number < max))

    You have to read all of the expressions to figure out what this is statement is checking for (and these aren't even very complicated expressions).  I would rather do this:

    bool isOdd = ((number % 2) != 0);
    bool isPositive = number > 0;
    bool isLessThanMax = number < max;
    if (isOdd && isPositive && isLessThanMax)

    Giving each of the subexpressions names helps me give more information to the person reading the code.  This makes the code longer, but for me it's a tradeoff that's worth it.

    If some has a problem to read  "number > 0" as "number is positive" or "number < max" as "number is less than max", you should not let him work on your code.
    "isOdd" might be a borderline case. Anyway, why not, just to be sure, simply add a comment?

    if (((number % 2) != 0) && (number > 0) && (number < max))  // number is odd and within limits
     



  • @ammoQ said:

    @burnmp3s said:

    if (((number % 2) != 0) && (number > 0) && (number < max))

    You have to read all of the expressions to figure out what this is statement is checking for (and these aren't even very complicated expressions).  I would rather do this:

    bool isOdd = ((number % 2) != 0);
    bool isPositive = number > 0;
    bool isLessThanMax = number < max;
    if (isOdd && isPositive && isLessThanMax)

    Giving each of the subexpressions names helps me give more information to the person reading the code.  This makes the code longer, but for me it's a tradeoff that's worth it.

    If some has a problem to read  "number > 0" as "number is positive" or "number < max" as "number is less than max", you should not let him work on your code.
    "isOdd" might be a borderline case. Anyway, why not, just to be sure, simply add a comment?

    if (((number % 2) != 0) && (number > 0) && (number < max))  // number is odd and within limits

    I write code the way I do to avoid those kind of comments.  That way its always clear what the code is doing, so that I can save the comments for explaining why its doing what its doing.



  • @burnmp3s said:

    I write code the way I do to avoid those kind of comments.  That way its always clear what the code is doing, so that I can save the comments for explaining why its doing what its doing.

    Valid point. Anyway, too many trivial abstractions like "isPositive" IMO make the program hard to read; "number>0" is perfectly well-defined in the programming language, but isPositive could as well mean "number>=0" if a programmer thinks so. I.e. you always have to check what "isOdd", "isPositive" etc. actually mean if you are trying to track an error.



  • @ammoQ said:

    I can aggree to that; my example is, after careful consideration, probably not a good one, especially since = in an if statement easily leads to misinterpretations. Let's make it

    entry = Phonebook.Lookup(whatever.Param["name"+foobar.Id]);
    if (entry
    != null) {
      ...
    }

    But that's why you don't do the IsListed test in such a case  -- just do the lookup, and catch the exception if the lookup failed.

     I for one love exceptions, and I use them exactly like burnmp3s.

    I have a question about exceptions in .NET bubbling up to the user though. I'm used to Delphi, where uncaughts exceptions (when thrown in an event) are caught by a default exception handler in the message loop. This exception handler displays an error message to the user, then continues the message loop. This has aborted the event handler, but continues the application so the user can try again, which in most cases is exactly what I want. However in .NET, at least in C#, the uncaught exception is never caught and causes the application to abort entirely.

    Does anyone know how can I achieve the same behaviour in C# as in Delphi? I've tried hooking some events that the documentation indicated might do the trick (such as OnThreadException), but none of them did the trick.



  • @RiX0R said:

    @ammoQ said:

    I can aggree to that; my example is, after careful consideration, probably not a good one, especially since = in an if statement easily leads to misinterpretations. Let's make it

    entry = Phonebook.Lookup(whatever.Param["name"+foobar.Id]);
    if (entry
    != null) {
      ...
    }

    But that's why you don't do the IsListed test in such a case  -- just do the lookup, and catch the exception if the lookup failed.

    I don't like that idea for several reasons. First, exceptions are for exceptional circumstances only.

    Second, the program might look like that if we follow your proposal:

    try {
      entry = Phonebook.Lookup(whatever.Param["name"+foobar.Id]);
      processEntry(entry);
    }
    catch (NoSuchEntryException e) {
      // can be safely ignored
    }
     

    But this is bad since a NoSuchEntryException thrown by processEntry() would be catched and ignored too, though we probably don't want to do that.

    So we end up writing code like that: 

    try {
      entry = Phonebook.Lookup(whatever.Param["name"+foobar.Id]);
    }
    catch (NoSuchEntryException e) {
      entry = null;
    }

    if (entry!=null) {
      processEntry(entry);
    }

    which is much longer and harder to read...



  • @KenW said:

    @RiX0R said:
    But that's why you don't do the IsListed test in such a case  -- just do the lookup, and catch the exception if the lookup failed.

     I for one love exceptions, and I use them exactly like burnmp3s.

    Arghhh... Don't do this!

    A simple test using Delphi's CPU window and some very simple code shows why:

    SL := TStringList.Create;
    SL.Add('Test line one');
    SL.Free;

    This generates the following assembly (with the source inline for reference): 

    Unit1.pas.29: SL := TStringList.Create;
    00452159 B201             mov dl,$01
    0045215B A180204100       mov eax,[$00412080]
    00452160 E8A316FBFF       call TObject.Create
    00452165 8BD8             mov ebx,eax
    Unit1.pas.30: SL.Add('Test line one');
    00452167 BA84214500       mov edx,$00452184
    0045216C 8BC3             mov eax,ebx
    0045216E 8B08             mov ecx,[eax]
    00452170 FF5138           call dword ptr [ecx+$38]
    Unit1.pas.31: SL.Free;
    00452173 8BC3             mov eax,ebx
    00452175 E8BE16FBFF       call TObject.Free

    This slight change to set up an exception stack:

    SL := TStringList.Create;
    try
      SL.Add('Test line one');
      SL.Free;
    except
      raise;
    end;

    causes the assembly code to change more than slightly:

    Unit1.pas.29: SL := TStringList.Create;
    0045215E B201             mov dl,$01
    00452160 A180204100       mov eax,[$00412080]
    00452165 E89E16FBFF       call TObject.Create
    0045216A 8BD8             mov ebx,eax
    Unit1.pas.30: try
    0045216C 33C0             xor eax,eax
    0045216E 55               push ebp
    0045216F 6897214500       push $00452197
    00452174 64FF30           push dword ptr fs:[eax]
    00452177 648920           mov fs:[eax],esp
    Unit1.pas.31: SL.Add('Test line one');
    0045217A BAB4214500       mov edx,$004521b4
    0045217F 8BC3             mov eax,ebx
    00452181 8B08             mov ecx,[eax]
    00452183 FF5138           call dword ptr [ecx+$38]
    Unit1.pas.32: SL.Free;
    00452186 8BC3             mov eax,ebx
    00452188 E8AB16FBFF       call TObject.Free
    0045218D 33C0             xor eax,eax
    0045218F 5A               pop edx
    00452190 59               pop ecx
    00452191 59               pop ecx
    00452192 648910           mov fs:[eax],edx
    00452195 EB0F             jmp $004521a6
    00452197 E9641BFBFF       jmp @HandleAnyException
    Unit1.pas.34: raise;
    0045219C E81B1FFBFF       call @RaiseAgain
    004521A1 E86A1FFBFF       call @DoneExcept

    Exceptions are called [b]exceptions[/b] because they're supposed to catch [b]exceptional[/b] things, not handle normal flow through code. In other words, they're for [b]unexpected[/b] things. To use the original phonebook example, it's not exceptional  (unexpected) to not find a name listed in the phone book; you could be in the wrong city, or the person could have an unlisted number, or they have a cell phone and no landline, or you've spelled it wrong, or the phone is listed in their wives/husbands name. I certainly don't have a stroke or heart attack when I try and look up a number and don't find it; that would be the equivalent of an exception to me. How about to you? :-)

    (Wow. By far the longest post I've ever made here. <g> 

    Try again removing the code tags, which I think are what caused the nbsp; insertions.



  • @RiX0R said:

    But that's why you don't do the IsListed test in such a case  -- just do the lookup, and catch the exception if the lookup failed.

    I for one love exceptions, and I use them exactly like burnmp3s.

    For the record I don't actually use exceptions this way.  The reason why I write functions like IsListed is to avoid having to do a try/catch to figure out if the code will throw an exception or not.  For example, I will generally check if a file exists before opening it rather than just opening it and catching the FileNotFound error.

    A lot of the exceptions that I throw in my code are never actually thrown during the execution of my programs.  Throwing the EntryNotFound exception is a design decision for my PhoneBook class, but it makes sense to put it in even if I know I'm going to always use IsListed to make sure it never actually gets thrown.  I know that might sound strange, but look at the .NET framework.  There are many exceptions in the .NET classes are not really meant to be caught, they are just there in case you use the classes incorrectly.

    Basically as I see it there are three ways to handle the Lookup failures:

    1.  Assume the client code will never try to lookup an entry that is not in the phonebook (after all, they can always check with IsListed).  You could document this prominently in the function header.  Inevitably somebody will put in code that forgets to check if an entry is listed, and you will get undefined behavior inside Lookup.  This will most likely be difficult to debug and might do damage to the phonebook data.

    2.  Return null if the entry is not listed.  Now the client has a choice: they can use IsListed so that Lookup never actually returns null, or they can forget about IsListed and just check to see if they get a null back.  Again though, eventually someone will forget to check for null and the client code will get an unexpected null back.  This will result in undefined behavior  in the client code.  Most of the time you will get a null reference exception in one of the next few lines, but sometimes the null can propagate and be very hard to track down.

    3.  Throw an exception if the entry is not listed.  This time the client code can use IsListed to avoid the exception, or it can just forget about IsListed and check to see if it throws the exception.  This time, if someone forgets to catch the exception, you either get an unhandled exception that crashes the program or (more likely) a high level catch block in the UI will tell the user that a missing phonebook entry caused the operation to abort.  There is no chance of undefined behavior.  Also, debugging is easy because you get a specific exception with a stack trace that tells you exactly what went wrong.

    I almost always choose option 3.  I write my code so that it avoids causing exceptions whenever possible, and the vast majority of my code contains no try/catch blocks.  If an exception is thrown, either because of a bug or because I purposely planned to have it thrown in that situation, I have a safety net in the form of my high level exception handlers.  I can't stress enough how effective this is for debugging, especially for debugging problems users are having.  I usually write all of my exceptions to my support logs, so that if a user finds a bug that I can't duplicate I can just ask for the support logs. 

     



  • @burnmp3s said:

    3.  Throw an exception if the entry is not listed.  This time the client code can use IsListed to avoid the exception, or it can just forget about IsListed and check to see if it throws the exception.  This time, if someone forgets to catch the exception, you either get an unhandled exception that crashes the program or (more likely) a high level catch block in the UI will tell the user that a missing phonebook entry caused the operation to abort.  There is no chance of undefined behavior.  Also, debugging is easy because you get a specific exception with a stack trace that tells you exactly what went wrong.

    Generally, in Java and .net, there is no "undefined behavior". If the phonebook returns null, and the calling program forgets the check, it will inevitably throw a null reference exception when it tries to access the object the first time.


    I almost always choose option 3.  I write my code so that it avoids causing exceptions whenever possible, and the vast majority of my code contains no try/catch blocks.  If an exception is thrown, either because of a bug or because I purposely planned to have it thrown in that situation, I have a safety net in the form of my high level exception handlers.  I can't stress enough how effective this is for debugging, especially for debugging problems users are having.  I usually write all of my exceptions to my support logs, so that if a user finds a bug that I can't duplicate I can just ask for the support logs. 

    If this is a multthreaded multiuser application, this option is prone to race conditions. Sooner or later, the entry will be removed by another thread between IsListed (true) and GetEntry (Exception); or it will be created between IsListed (false) and AddEntry (Exception).

     



  • Multi-threaded... that's what locking is for :)



  • @XIU said:

    Multi-threaded... that's what locking is for :)

    Exactly. But since the calling program executes two connected methods on the PhoneBook, it has to take care of that itself.

    So we are coming to

    synchronized(Phonebook) {
      try {
        entry = Phonebook.Lookup(whatever.Param["name"+foobar.Id]);
      }
      catch (NoSuchEntryException e) {
        entry = null;
      }

      if (entry!=null) {
        processEntry(entry);
      }
    }

    and this guarantees correct behaviour only if DeleteEntry is a synchronized method. 



  • @ammoQ said:

    Generally, in Java and .net, there is no "undefined behavior". If the phonebook returns null, and the calling program forgets the check, it will inevitably throw a null reference exception when it tries to access the object the first time.

    Like I said, most of the time you will get a null reference exception right after the buggy call, but not always.  For example, the null entry might be passed to a function that accepts null as a parameter.  My point is that the fact that you get a null reference exception is not gauranteed, and the few times that you don't can be very nasty. 

    @ammoQ said:

    If this is a multthreaded multiuser application, this option is prone to race conditions. Sooner or later, the entry will be removed by another thread between IsListed (true) and GetEntry (Exception); or it will be created between IsListed (false) and AddEntry (Exception).

    We already talked about this, I would use locking in this case.  I would probably do this by treating the phonebook as a resource, by implementing IDisposable so that you can use "using" to lock and unlock the phonebook like a file.  Files are a good analogy, because they have the same issue (you can get FileExists (true) and still get FileOpen (Exception)).  Also, if you are using it for a "multithreaded multiuser application", it sounds like you are just wanting ACID transactions.  If so, I could just put the phonebook data in a database and let the database worry about most of the concurrency issues.

    Anyway, all of this is getting far from my point:  If I have a choice between returning null to represent an error and throwing an exception to represent an error, I will usually throw the exception.
     


Log in to reply