3

Pretty new to C#, and I'd like to know how should I be using exceptions? I mean, not a mechanics level, but at a good practice level.

I'll use for example my calculator which tokenizes and the converts to RPN and solves problems given in RPN.

During the tokenizing step there's various invalid inputs, say "7.7.8" or "^&##", should I have separate exceptions for unknown symbols and invalid numbers? Is it wrong to say have a single exception and then a method in it containing the kind of error, to be given to the user?

I've really not been able to find much material on this kind of thing, so I thought I'd ask people with more experience than me.

-----Edit:

Thanks everyone for your awesome responses, I learned a ton today :) about my question, and even more for that matter.

Matthew Blanchard
  • 508
  • 6
  • 11
  • 2
    Exceptions are expensive and inputs should be validated. If an invalid input is detected you need to handle that in your code without throwing an exception. – Dean Kuga Mar 17 '11 at 19:42
  • 3
    @kzen I disagree. If invalid input is provided, you should throw an ArgumentException back to the caller and let the caller handle it. Exceptions might be expensive, but for a simple calculator application, throwing one for invalid input is pretty standard. – Scott Arrington Mar 17 '11 at 19:46
  • 1
    @kzen - depends on where the input is coming from. from a user, yeah you should validate and fail gracefully. from another block of code, that code is screwed up and exceptions are the way to inform the developer – Robert Levy Mar 17 '11 at 19:46
  • The use of exceptions in this case seems to greatly simplify the flow of the program, I handle the invalid input with an exception. Expensive doesn't particularly matter here. – Matthew Blanchard Mar 17 '11 at 19:47
  • @Robert, that is a valid addition to my original statement... – Dean Kuga Mar 17 '11 at 19:48
  • Since you're new to C#, here is something you should also be aware of: [Do not rethrow exceptions](http://blogs.msdn.com/b/ericlippert/archive/2010/03/04/too-much-reuse.aspx) . – Brian Mar 17 '11 at 21:55

8 Answers8

15

Think about your lexer from the perspective of preconditions and postconditions. Here are two ways to design a lexer :

  1. A lexer is a device which takes in a well-formed input string and outputs a lexical analysis. The input string is required to be well-formed; if it is not then the lexical analyzer fails with an exception.

  2. A lexer is a device which takes an input string and produces a lexical analysis. Lexical analysis includes identification of and analysis of error conditions.

Do you know that the string you're going to pass in is correct, and you want an exception in the exceptional circumstance that it is incorrect? Or do you not know that the string is correct, and you wish the lexer to determine that?

In the former case, throw an exception. In the latter case, make the error analysis part of the output of the lexer.

Basically what I'm saying is do not produce vexing exceptions. Vexing exceptions are extremely irritating. If the caller of your code is calling it to determine whether the string is correct then they don't want that information in the form of an exception, they want it in the form of a data object that represents the error analysis. If the purpose of the code is to produce an error analysis then produce such an analysis as the normal operation of the method. Do not embed the analysis in an exception and make someone trap the exception in order to get it.

Eric Lippert
  • 647,829
  • 179
  • 1,238
  • 2,067
3

In my experience you should not use exceptions as part of the logic or control flow of the code. You should use it when there is a true exception. In your case, that would be, for instance, when there is an illegal character, character which shouldn't be there in the first place. If, however, the function is validating the string, you should not throw an exception when you find an illegal character, at least that is my opinion.

I use exceptions when database or file that "should" be there cannot be found, when a key/pair value is missing when it ought not to be etc.

bjorsig
  • 1,077
  • 2
  • 7
  • 20
  • I agree with this statement for the most part, however invalid input into a method is an Exception case in my opinion if there's no way to accurately fix the invalid input. If your method cannot give the user a valid answer because they supplied invalid input, then you should throw an Exception so they know about it instead of just returning `null` or something else equally meaningless. – Scott Arrington Mar 17 '11 at 19:51
  • an exception is a very bad way to tell a user about an invalid input, and if it is unhandled the application will just crash. If you handle it, well why throw in the first place? just validate the input and display a nice message to the user. – Joel Gauvreau Mar 17 '11 at 19:59
  • But if your application is separated into layers, you cannot just validate and display a message to the user, because the layer looking at the input has no knowledge of how to display that to the user. Also, if your method is `public int Calculate(string value)`, how do you signify that the number being returned is incorrect and is invalid? – KallDrexx Mar 21 '11 at 18:21
  • @KallDrexx See Eric Lippert post on Vexing exception : http://blogs.msdn.com/b/ericlippert/archive/2008/09/10/vexing-exceptions.aspx Int.Parse and Int.TryParse are perfect example of this.. – Joel Gauvreau Mar 22 '11 at 19:46
  • No I read that, but I don't think that clears up the issue though. I don't personally feel it's good practice to make all of my service layer methods `boolean` with an out parameter, just so I can avoid throwing an exception when things are not as expected. It not only adds code to the calling function, but makes it a lot harder to cascade errors through multiple service layers. If Method A calls Method B which calls Method C, if Method C fails due to data passed in originally from Method A, method B may not care that the error happened but you are forced to write code to handle it in B. – KallDrexx Mar 22 '11 at 20:00
  • I somewhat agree, but you are confusing the issue. The question was about validating user input...I concede that there are some case where there is no easy way out of it. The rules are more guidelines than actual rules. :) – Joel Gauvreau Mar 22 '11 at 20:06
  • @KallDrexx One way I handled that in the past was by returning a Result<> object that contains some additional information along side the return value. – Joel Gauvreau Mar 22 '11 at 20:09
  • All programming is guidelines instead of rules :). Although I still think validating user input is better either with exceptions or with your `Result<>` idea, because I think you still need to inform the user why it's not valid, instead of just saying it's invalid. Not using exceptions makes sense for `Int.TryParse` because it obviously failed because it's not an integer, but there are many other situations where the failure reason isn't clear. – KallDrexx Mar 22 '11 at 20:13
  • It is very hard to generalise and say never do this, or always handle it that way. In the calculate method, you can validate before hand that the string contains only valid characters. It is much harder to validate if the actual calculation make sense without reimplementing the whole thing just to check if it is valid or not, in that case, just try it and throw an exception. Some calculation may take a long time to run, especially if it goes through many service layers, so you should validate what you can first. – Joel Gauvreau Mar 22 '11 at 20:32
  • 1
    The point is if you have a routine public SqlConnection(string connectionString) then the function should throw an error if the string is not correctly formatted connection string, but if the routine is public SqlConnection CheckConnectionString(string connectionString) then it should definitely not throw an exception. Exceptions should be thrown when something happens which is not foreseen in the program. It should not be used to inform users about status of things, strings, number, formats or anything else. I think that is a good rule of thumb to use. – bjorsig Mar 22 '11 at 21:45
2

Create exceptions for programmers, not users. Think what would be useful to a programmer calling your method?

If there is specialized error handling code they should consider writing for a specific cause of an exception, then provide a specific type of exception so they can have a 'catch' block specifically for it.

If the error handling code they should write is generic across various exception causes, then provide a more general type exception so they only have to code against that one.

Robert Levy
  • 28,747
  • 6
  • 62
  • 94
1

This is a good reference on the best practices of exception handling in .Net.

http://www.codeproject.com/KB/architecture/exceptionbestpractices.aspx

Brandon Moretz
  • 7,512
  • 3
  • 33
  • 43
  • I have to disagree with, "Use exceptions for errors that should not be ignored." The example for this guideline is a classic example of using exceptions to handle control flow. – Brian Mar 17 '11 at 22:01
1

A rule of thumb I like for determining if an exception is appropriate: If a function promises to do something and does not do it, it should throw an exception. A simple example would be Parse vs TryParse. Notice that the Parse functions promise to parse a string, then throw an exception if they cannot (e.g., because the string is in the wrong format). TryParse promises to try to parse, so it does not throw exceptions in if it cannot parse. In short, if your function makes a promise and fails to fulfill it, you should throw an exception.

Alternatively: do not throw an exception unless the preconditions of your function are unsatisfied. I interpret Eric Lippert's Vexing Exceptions as a discussion on what types of preconditions are inappropriate.

Brian
  • 25,523
  • 18
  • 82
  • 173
0

Exception should be for exceptional case, user input is not one of those case.

This question might help you When to throw an Exception

Use validation instead : Winform ui validation

Community
  • 1
  • 1
Joel Gauvreau
  • 3,586
  • 4
  • 29
  • 32
0

The general rule is KISS. You should only throw/create exceptions if you have need. There is no need to create a whole suite of exceptions if no one is every going to catch it.

In your case you could

  1. Throw an InvalidArgument or create your own indicating that it is invalid user input.
  2. Give a detailed error messages explaining what the problem was.

If you find that you are catching the exception and parsing the message and taking actions then its time to create custom exception/exceptions.

You could create a custom exception that has a "type" field, but any type of design that is dependent on exceptions is a bad one.

Nix
  • 57,072
  • 29
  • 149
  • 198
0

all of your parsing exceptions should at least share a common base class so that the caller can catch any parsing exception without listing them all in the catch clause (and potentially forgetting one).

you can go down the road of making a taxonomy of subclasses but i wouldn't bother making such explicit distinctions until you have a need to do so. if the calling program takes the same action for all parsing errors then they can all be of the same class, varying only by display message. if it becomes clear that certain errors need to be handled differently, make a subclass. design around use cases.

as an aside: i think a given method should throw an exception if it cannot deliver what it promises, thus protecting the developer from assuming success in the case of failure. deciding which promises to make is a greater design question and you can look at existing parsing APIs for inspiration.

user648818
  • 1