Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Throw extra exception to avoid code duplication

First of all, I know the standard answer will be that exceptions are never to be used for flow control. While I perfectly agree with this, I've been thinking a long time about something I sometimes did, which I'll describe with the following pseudo-code:

try
    string keyboardInput = read()
    int number = int.parse(keyboardInput)
    //the conversion succeeds
    if(number >= 1000) 
        //That's not what I asked for. The message to display to the user
        //is already in the catch-block below.
        throw new NumberFormatException() //well, there IS something wrong with the number...
 catch(NumberFormatException ex)  //the user entered text
    print("Please enter a valid number below 1000.")

First of all, take this example in a very abstract way. This does not necessarily have to happen. The situation simply is:

A user input needs to be constrained and can go wrong in 2 ways, either by a thrown exception the language defines, or by a check. Both errors are reported by the user in the same way, because they do not need to know the technical difference of what caused it.

I have thought of several ways to solve it. To begin with, it would be better to throw a custom made exception. The problem I then face is, if I catch it locally, what to do with the other exception? In se, the custom exception would be cause for a second catch-block, in which the message would be copied into just as well. My solution:

//number is wrong
throw new MyException()
catch(NumberFormatException ex) 
    throw new MyException()
catch(MyException ex) {
    print("Please enter...")

The meaning of the exceptions' names is everything here. This application of custom-made exceptions is widely accepted, but essentially I didn't do anything different from the first way: I forced to go into a catch-block, albeit by throwing a custom exception rather than a standard-library one.

The same way applied to throwing the exception on to the calling method (thus not having a catch block for the custom exception) seems to make more sense. My method can go wrong in what is technically two ways, but essentially one way: wrong user input. Therefore, one would write a UserInputException and make the method throw this. New problem: what if this is the main method of an application?

I'm not currently struggling with a specific application to implement this kind of behaviour, my question is purely theoretical and non-language specific.

What is the best way to approach this?

like image 949
MarioDS Avatar asked May 09 '12 09:05

MarioDS


People also ask

How do you avoid code duplication?

Don't Repeat Yourself (DRY): Using DRY or Do not Repeat Yourself principle, you make sure that you stay away from duplicate code as often as you can. Rather you replace the duplicate code with abstractions or use data normalization. To reduce duplicity in a function, one can use loops and trees.

How do I stop code duplication in HTML?

If you want to keep a consistent html structure for the sidebar on each page, use php. You would create a file called sidebar. php, and use the include method to import the same code into each page. If you want to change it later, edit the single sidebar.

Does abstraction avoid code duplication?

Abstractions Don't Solve Duplication We as engineers have learned how to use abstraction to solve duplication problems. However this doesn't mean that every such problem requires an abstraction. We design one based on the current state of the code but that may be misleading.


6 Answers

I would consider the first exception to be low-level, and I would handle it (by translation in this case) at the point of call. I find that this leads to code that is easier to maintain and refactor later, as you have less types of exceptions to handle.

try
  string keyboardInput = read()
  try
    int number = int.parse(keyboardInput)
  catch(NumberFormatException ex)
    throw MyException("Input value was not a number")

  //the conversion succeeds
  if(number >= 1000) 
    throw MyException("Input value was out of range")

catch(MyException ex)  //the user entered text
  print( ex.ToString() )
  print("Please enter a valid number below 1000.")
like image 112
Michael Anderson Avatar answered Oct 05 '22 04:10

Michael Anderson


I think you have essentially a few ways to go about it with minimal code duplication in mind:

  1. Use a boolean variable/store the exception: If there was an error anywhere in the the general logic of the specific task you are performing, you exit on the first sign of error and handle that in a separate error handling branch.

    Advantages: only one place to handle the error; you can use any custom exception/error condition you like.

    Disadvantages: the logic of what you are trying to achieve might be hard to discover.

  2. Create a general function that you can use to inform the user about the error (pre-calculating/storing all information that describes the general error, e.g. the message to display the user), so you can just make one function call when an error condition happens.

    Advantages: the logic of your intent might be clearer for readers of the code; you can use anu custom exception/error conditon you like.

    Disadvantages: the error will have to be handled in separate places (although with the pre-computed/stored values, there is not much copy-paste, however complex the informing the user part).

  3. If the intent is clear, I don't think throwing exceptions from within your try block explicitly is a bad idea. If you do not want to throw one of the system provided exceptions, you can always create your own that derives from one of them, so you only need a minimal number (preferably one) of catch blocks.

    Advantages: only one place to handle error condition -- if there is essentially only one type of exception thrown in try-block.

    Disadvantages: if more than one type of exception is thrown, you need nested try-catch blocks (to propagate the exceptions to the most outward one) or a very general (e.g. Exception) catch block to avoid having to duplicate error reporting.

like image 21
Attila Avatar answered Oct 05 '22 04:10

Attila


The way I see it is this:

Assuming there's no other way to parse your int that doesn't throw an exception, your code as it is now, is correct and elegant.

The only issue would be if your code was in some kind of loop, in which case you might worry about the overhead of throwing and catching unnecessary exceptions. In that case, you will have to compromise some of your code's beauty in favor of only handling exceptions whenever necessary.

error=false;

try {
    string keyboardInput = read();
    int number = int.parse(keyboardInput);
    //the conversion succeeds
    if(number >= 1000) {
        //That's not what I asked for. The message to display to the user
        //is already in the catch-block below.
        error=true;
} catch(NumberFormatException ex) { //the user entered text
    error=true;
}

if (error)
    print("Please enter a valid number below 1000.");

Also you can think about why you're trying to aggregate two errors into one. Instead you could inform the user as to what error they did, which might be more helpful in some cases:

try {
    string keyboardInput = read();
    int number = int.parse(keyboardInput);
    //the conversion succeeds
    if(number >= 1000) {
        //That's not what I asked for. The message to display to the user
        //is already in the catch-block below.
        print("Please enter a number below 1000.");

} catch(NumberFormatException ex) { //the user entered text
    print("Please enter a valid number.");
}
like image 28
alexg Avatar answered Oct 05 '22 04:10

alexg


You do not need any exceptions in this particular example.

int number;
if (int.TryParse(keyboardInput, out number) && number < 1000) // success
else // error

However, the situation you describe is common in business software, and throwing an exception to reach a uniform handler is quite common.

One such pattern is XML validation followed by XSLT. In some systems, invalid XML is handled through catching validation exceptions. In these systems, it is pretty natural to reuse the existing exception handling in XSLT (which can naturally detect some classes of data errors that a particular validation language cannot):

<xsl:if test="@required = 'yes' and @prohibited = 'yes'>
    <xsl:message terminate='yes'>Error message</xsl:message>
</xsl:if>

It is important to see that if such conditions are extremely rare (expected to occur only during early integration testing, and disappear as defects in other modules get fixed), most of the typical concerns around not using exceptions for flow control do not really apply.

like image 20
Jirka Hanika Avatar answered Oct 05 '22 04:10

Jirka Hanika


What about approaching this validation problem by writing several validator classes that take in an input and return errors, or no errors. As far as your struggle with exceptions: put that logic into each validator and deal with it there on a case by case basis.

after that you figure out the correct validators to use for your input, collect their errors and handle them.

the benefits of this are:

  1. Validators do one thing, validate a single case
  2. Its up to the validation function to decide how to handle the errors. Do you break on first validation error or do you collect them all and then deal with them?
  3. You can write your code is such a way that the main validation function can validate different types of input using the same code, just picking the correct validators using your favorite technique.

and disadvantages:

  1. You will end up writing more code (but if you are using java, this should be put into the 'benefits' bucket)

here is some example pseudo-code:

validate(input):
   validators = Validator.for(input.type)
   errors = []
   for validator in validators:
       errors.push(validator.validate(input))

   if errors:
       throw PoopException          

and some validators:

MaxValidator extends IntValidator: 
    validate(input):
        errors = []
        errors.push(super.validate(input))
        if input > 1000:
            errors.push("bleee!!!! to big!")
        return errors

IntValidator:
     validate(input):
        try:
           int.parse(input)
        catch NumberFormatException:
           return ['not an int']
        return []

of course you would need to do some trickery to make the parent validator possibly return you a valid version of the input, in this case string "123" converted to an int so the max validator can handle it, but this can be easily accomplished by making the validators statefull or some other magic.

like image 39
mkoryak Avatar answered Oct 05 '22 05:10

mkoryak


I can't see this answer anywhere in here, so I'll just post it as another point of view.

As we all know, you can actually break the rules if you know them well enough, so you can use throwing an Exception for flow control if you know it's the best solution for your situation. From what I've seen, it happens usually with some dumb frameworks...

That said, before Java 7 (which brought us the mighty multicatch construct), this was my approach to avoid code repetition:

try {
    someOffendingMethod();
} catch (Exception e) {
    if (e instanceof NumberFormatException || e instanceof MyException) {
        System.out.println("Please enter a valid number.");
    }
}

It's a valid technique in C#, too.

like image 29
Petr Janeček Avatar answered Oct 05 '22 06:10

Petr Janeček