Cease throwing bare Exception #1498
Conversation
| public static double Sqrt(this double numericProperty) | ||
| { | ||
| throw new Exception("Not to be used directly - use inside QueryOver expression"); | ||
| throw new InvalidOperationException("Not to be used directly - use inside QueryOver expression"); |
There was a problem hiding this comment.
I think it's better to wrap this exception creation in some method to minimize copy-paste for future extensions/refactorings. So it should look something like:
throw DirectUsageException();
There was a problem hiding this comment.
Does it have to throw at all? Real methods are replaced in queries all the time, so why not here, i.e call Math.Sqrt and be done with it?
There was a problem hiding this comment.
@gliljas could do, but in another RP.
There was a problem hiding this comment.
Does it have to throw at all? Real methods are replaced in queries all the time, so why not here, i.e call Math.Sqrt and be done with it?
Some cases could be trickier, like the Projection.Concat where string.Concat null handling will not be similar to SQL default null handling. So this may cause more troubles than simply throwing.
teamcity.build
Outdated
| if (brokenTests.Count > 0) | ||
| throw new Exception("Previously passing tests have been broken\n\n" + output); | ||
| throw new InvalidOperationException("Previously passing tests have been broken\n\n" + output); |
There was a problem hiding this comment.
This is not InvalidOperationException. This is either need to stay as an Exception or another exception type, probably from Nant's namespace.
There was a problem hiding this comment.
Well, reverting. This script is currently no more used so changing it is not tested.
| { | ||
| if (value != null) | ||
| throw new Exception("null component property"); | ||
| throw new InvalidOperationException("null component property"); |
There was a problem hiding this comment.
Should be ArgumentException/ArgumentNullException
|
I have made more changes than the ones requested in review, especially in |
4 participants
Throwing the
Exceptionclass is a bad practice. We should throw at least slightly more specific exceptions.