Method which does nothing or throw Exception

Question

I have a big method, part of which checks the state of an object and throws an exception if the state is invalid. Should I extract a method? The new method 'CheckState' will do nothing or throw an exception.

Answer 1

The convention is for Check— to return bool, whilst Verify— would throw exception if the verification fails.

See, for example, Dispatcher.CheckAccess and Dispatcher.VerifyAccess:

The difference between CheckAccess and VerifyAccess is CheckAccess returns a Boolean indicating whether the calling thread has access to the Dispatcher and VerifyAccess throws an exception.

Answer 2

If I understand you correctly, you mean to say that part of your method checks the state of an object and throws an exception if it is invalid.

You are further asking whether you should move this to its own method(one that checks the state of an object and throws an exception of it is invalid).

The answer really is; probably neither. An exception should really only be thrown in "exceptional" circumstances. If you enter your method and expect the object to be in a valid state, then use it as if it were.

If something occurs that is unexpected, then catch that exception and throw your "InvalidStateException". (If programmed properly this shouldn't even be necessary either.)

If you enter your method and are not sure that your object is in a valid state, then it being in an invalid one is not "unexpected" behavior, and it should be handled differently.

This is where your Check method would come into place. It shouldn't throw an exception but should probably return a boolean, which will determine what you do next. An invalid object in your case is perfectly reasonable, so use your code to handle that case with a check method that returns its valid_state boolean.

This and this describe what I am talking about.

Answer 3

It's even good practice to separate the state-checking from the state-changing code. However, if your class is relying much on state here and there, you should probably take a look at the State Design Pattern.

In this pattern, the difference in behavior is modeled by using a method, implemented differently for each State class.

This may be implemented better than following, but it gives you a taste:

class FooState {
   FooState handleFoo();
   FooState handleBar();
}

class ValidState {
   FooState handleFoo(){...
   }
   FooState handleBar(){
      return InvalidState(stateful);
   }
}

class InvalidState {
   FooState handleFoo() { throw InvalidState(); }
   FooState handleBar() {

       return ValidState(stateful);
   }
}

class StatefulObject {
   public FooState state;
   public void foo(){ state=state.handleFoo(); }
   public void bar(){ state=state.handleBar(); }
}

Answer 4

If it truly does nothing, then you should be able to safely remove the code completely.

Otherwise, the code MUST do something. In that case then you should be able to safely create a separate method which will most certainly make your code cleaner.