You have a problem. You see a symptom of it in the difficulty in testing the methods behind DoRejection()
, but that is only a symptom of the problem, not the problem itself.
The underlying problem is that you have a method that hides sequential coupling.
A co-worker of mine (a few employers ago) had a saying:
If you use the word 'and' when describing what the method does, you need to break this method into smaller parts.
Another way of saying this is "You've violated the single responsibility principle for this method." It is doing too many things. The method does not simply "do a rejection", but it rather:
- updates the part status
- removes the rejected part
- clears the rejected part data
- creates a new part
- commits the rejection with comments
This are far too many things that are going on behind the scene. If this is part of a transaction, expose the transaction. The reason you are having difficulty testing the parts in isolation is because they are not in isolation. The state of removing the rejected part data requires that the respected part be removed.
A classic approach to this, as part of exposing the methods, is to impose a design by contract. Check the preconditions, and if they are not met, raise an error.
You could also impose the sequence by returning an object that wraps the underlying class and only presents the appropriate next states.
This would be especially useful if you wanted to be able to update multiple parts but only do this as one commit.
There are other problems that creep into this that have corresponding code smells. Methods that take no arguments (and return no values), especially when sequential coupling is in play, are acting on the object state as a pseudo-global value. There are methods that indicate that there are side effects that the next method depends on. This raises some very serious questions about the underlying design and has a very strong code smell to them.
Its hard to say exactly where the problem is from this small sample of code, but it is fairly clear that there is a significant problem with the design foundations upon which this code is based.