views:

120

answers:

6

Lets say have this immutable record type:

public class Record
{
    public Record(int x, int y) {
        Validator.ValidateX(x);
        Validator.ValidateY(y);
        X=x;
        Y=y;
    }

    public final int X;
    public final int Y;

    public static class Validator {
        public void ValidateX(int x) { if(x < 0) { throw new UnCheckedException; } }
        public void ValidateY(int y) { if(y < 0) { throw new UnCheckedException; } }
    }
}

Notice it throws an unchecked exception. The reason is because this is a object that is used quite often and it is inconvenient to have to deal with a checked exception.

However, if this object is in a class library where it may be used to validate user inputs (or some other external input). Now it's starting to sound like it should be a CHECKED exception, because the inputs are no longer up the to programmer.

Thoughts everyone? should i make checked or unchecked, or are there better design for this?

UPDATE:

my confusion is coming from this scenerio: normally Record would be used like this:

Record r = new Record(1,2);
OtherObj o = new OtherObj(r);

there it's up to the programmer, so unchecked exception is ok.

However when you get parameters for Record from a user you want to validate them right? so you might call

Record.ValidateX(inputX);
Record.ValidateY(inputY);

There it might throw a checked exception because inputs are no longer controlled?

Sorry, I normally wouldn't be too concerned with this (personally I think unchecked is fine). but this is actually a problem in a homework and I want to get it right lol.

UPDATE(2): I starting to think what I need is for ValidateX to throw a checked exception because it is typically what would be used if user input is involved. In that case we could ask the user for input again. However, for the Record constructor, it will throw checked exception because constructing a Record with invalid arguements is an API violation. The new code would look like this:

public class Record
{
    public Record(int x, int y) {
        try
        {
            Validator.ValidateX(x);
            Validator.ValidateY(y);
        }catch(CheckedException xcpt) { throw new UnCheckedException(xcpt.getMessage()); }
        X=x;
        Y=y;
    }

    public final int X;
    public final int Y;

    public static class Validator {
        public void ValidateX(int x) throws CheckedException { if(x < 0) { throw new CheckedException; } }
        public void ValidateY(int y) throws CheckedException { if(y < 0) { throw new CheckedException; } }
    }
}

Now the programmer can validate the parameters before passing them to the Record class. If the does not then it's a API violation and an unchecked exception is thrown.

How does this sound?!

+2  A: 

Really, it's all about expectations. If you expect that under normal program flow an invalid input may be entered, it should be a checked exception so you can recover gracefully. Unchecked exceptions are for error cases that the programmer can't do anything about and doesn't expect to happen normally.

danben
+6  A: 

Notice it throws an unchecked exception. The reason is because this is a object that is used quite often and it is inconvenient to have to deal with a checked exception.

No, I think the reason you throw an unchecked exception here is because it involves a violation of contract on the part of the user of that API, and not an exceptional condition during the normal functioning of that method. See the Java SDK's use of NullPointerException, IllegalArgumentException, etc., which are RuntimeExceptions because they represent a violation of contract.

Jonathan Feinberg
This would be better if the contract were explicitly documented in the class documentation - but I'll assume they were omitted for brevity.
DJClayworth
+4  A: 

Bloch, item 58: Use checked exceptions for recoverable conditions and runtime exceptions for programming errors.

If the user of your class is able to avoid the problem by using the API correctly (including not passing in values that are documented to be invalid) then it's a programming error, so throw an InvalidArgumentException.

ptomli
+1  A: 

If this is really meant to be for a validator class, then I would expect it to be unusually flexible on its range of allowable input, simply because it's purpose for existence is to test validity, which it can't do nicely by throwing exceptions.

public interface Validator<T> {
    public boolean isValid(T object);
}

public final class PositiveIntegerValidator implements Validator<Integer> {
    public boolean isValid(Integer object) {
        return object != null && object > 0;
    }
}
ptomli
no this is just an example. the real problem has integers that have to be between certain values such as 0 > x < 100
cchampion
Can you correct the problem if you've got invalid values in hand? Such as asking the user via the UI for more suitable values.
ptomli
yes, you could correct them by asking for more suitable values
cchampion
+1  A: 

As a rule of thumb I always handle exceptions at the public interface to my component/layer/api. This allows exceptions to bubble up and be handled at the top level - Last Responsible Moment, whilst ensuring I handle the exception in my code, avoiding leaky exceptions.

BTW Some excellent advice regarding database exceptions on this question and general exception handling on this question

Paul
+1  A: 

You should never validate user input with an unchecked exception. Exceptions are normally checked because that way you can not forget to handle the exceptions.

Validating parameters used in an api is a completely different kind. These should be unchecked, because otherwise you'll end up adding try/catch to every function call. Whenever a method is passed an invalid argument, this is surely a programming error. The normal way is to throw an IllegalArgumentException or NullPointerException (or any other that suits your needs). In api calls leave the checked excpetions for

  1. Validations you promise to the caller of the api
  2. Expected exceptional conditions (requested file doesn't exist, write failed etc)

Besides the above, recovererability is also important for deciding for a checked or unchecked exception. If you can never recover (which is normally the case in a programming error) you can (or should?) take the unchecked excpetion.

Preferably don't write code that doesn't meet the above guidelines. For you that would mean that you have to write a function that returns a boolean or checked exception when using to validate user input, and an unchecked excpetion when validating input parameters to your functions. Of course your can and should use the same function to validate the exceptions, just wrap the one that returns a boolean or checked excpetion:

public boolean validateX(int x)
{
     return x > 0;
}

private void validateParameter(int x)
{
     if (validateX(x))
     {
         throw new IllegalArgumentException("X is invalid");
     }
}

This is a bit more work, but it will give you the best of both worlds. By making the validate parameter functino private you can ensure you don't accidently use it outside of your class. Of course you can also put the if(validateX(x)) ... part inside your constructor.

Thirler
I think you just advised me what I put in my latest update(2). check it out and let me know if i'm doing correctly what you described here!!! Thank yoU!!!
cchampion
Yes you are correct. As an extra tip: when rethrowing an exception as another exception. Use the constructor that takes an exception, that way more information (such as your stack) is preserved the new exception.
Thirler