I think you are on the right track. Neither throwing, catching, nor documenting all potentially throwable exceptions makes much sense. There are times where the product stringency requires a higher degree of exception employment and documentation (e.g. certain safety critical aspects of a systems).
The strategy of being more defensive, using contract concepts to identify preconditions (and postconditions) on especially downstream callers (e.g. anything that resembles a public or protected member) will often be more effective and more flexible. This applies not only to the implementation, but to the documentation. If developers know what is expected, they are more likely to follow the rules and less likely to be confused or misuse the code you have written.
Some of the common things that should be documented include the case of null parameters. Often there is a consequence for their use that drives the result to something that wouldn't normally be expected, but are allowed and used for a variety of reasons, sometimes for flexibility. As a consumer of a member that has parameters that allow null, or other special, non rational values (like negative time, or negative quantities), I expect to see them identified and explained.
For non null parameters, as a consumer of a public or protected member, I want to know that null is not allowed. I want to know what the valid range of values in the given context is. I want to know the consequences of using values that are outside the normal range, but are otherwise valid in a different calling context (eg the value of the type is generally valid for any operation, but not here -- like a boolean parameter that doesn't expect false as a valid value.
As far as platform, or otherwise well known interfaces, I don't think you have to go to extremes in documenting it. However, because you have an opportunity as a developer to vary the implementation from whatever platform guidance, making note of how it follows that guidance may be of value.
Specific to IDisposable, often implementations of this interface offers an alternative method that is preferred over the explicit disposal process. In these cases, highlight the preferred method, and note that the explicit disposal is not preferred.
Best Answer
You're right
The argument for your side is already mentioned by Robert Harvey: don't add code you don't need right now, especially since it's easy to add it later.
Your reviewer is right, too
On the other hand, the reviewer's point is understandable as well:
Returning a generic
Exception()
is not very helpful to the caller: while the exception description indicates to an human what's going on, treating exceptions differently programmatically may be impossible. The developer using your code may be reluctant changing the types of exceptions, including by fear (justified or not) of breaking something.Note that adding custom exceptions right now is not that difficult:
is all you need. That's only two lines of code (given that you may not even need to create a separate file if you put custom exceptions in one file).
The code itself looks better, more readable.
looks slightly more readable compared to:
because of the indication of the type of exception:
In the second piece of code, I need to read the description and guess that the price is out of range (or maybe it's not? Maybe there are cases where price can be negative, such as rebates?)
In the first piece of code, a glimpse over the type gives an immediate indication about the error. It looks like there is a set of allowed values for a price, and the current value is outside this set.
So?
So:
Both approaches are valid. If you don't subclass the exceptions when you don't need custom types, you're right. When you do subclass the exceptions because it costs nothing to do it and may be useful later, you're right to.
Be consistent with your team. If your team uses custom exceptions extensively, use them.