OOP Design Question - Validating properties

Question

I have the habit of always validating property setters against bad data, even if there's no where in my program that would reasonably input bad data. My QA person doesn't want me throwing exceptions unless I can explain where they would occur. Should I be validating all properties? Is there a standard on this I could point to?

Example:

public void setName(String newName){
   if (newName == null){
      throw new IllegalArgumentException("Name cannot be null");
   }
   name = newName;
}

...

//Only call to setName(String)
t.setName("Jim");

Answer 1

You are doing ok ! Whether it's a setter or a function - always validate and throw meaningfull exception. you never know when you'll need it, and you will...

Answer 2

It's a tradeoff. It's more code to write, review and maintain, but you'll probably find problems quicker if somehow a null name gets through.

I lean towards having it because eventually you find you do need it.

I used to have utility classes to keep the code to a minimum. So instead of

if (name == null) { throw new ...

you could have

Util.assertNotNull(name)

Then Java added asserts to the language and you could do it more directly. And turn it off if you wanted.

Answer 3

Personally I prefer using Asserts in these wildly improbable cases just to avoid difficult to read code but to make it clear that assumptions are being made in the function's algorithms.

But, of course, this is very much a judgement call that has to be made on a case-by-case basis. You can see it (and I have seen it) get completely out of hand - to the point where a simple function is turned into a tangle of if statements that pretty much never evaluate to true.

Answer 4

It's well done in my opinion. For null values throw IllegalArgumentException. For other kind of validations you should consider using a customized exceptions hierarchy related to your domain objects.

Answer 5

In general I don't favor this practice. It's not that performing validation is bad, but rather, on things like simple setters it tends to create more clutter than its worth in protecting from bugs. I prefer using unit tests to insure there are no bugs.

Answer 6

I'm not aware of any documented standard that says 'validate all user input' but it's a very good idea. In the current version of the program it may not be possible to access this particular property with invalid data but that's not going to prevent it from happening in the future. All sorts of interesting things happen during maintenance. And hey, you never know when someone will reuse the class in another application that doesn't validate data before passing it in.

Answer 7

You're enforcing your method's preconditions, which are an important part of its contract. There's nothing wrong with doing that, and it also serves as self-documenting code (if I read your method's code, I immediately see what I shouldn't pass to it), though asserts may be preferable for that.