alt Hacker NewsI really dislike this pattern:
try
{
id = int.Parse(inputId);
}
catch (Exception ex) when (ex is FormatException or OverflowException)
{
throw new InvalidOperationException("DeactivateUser failed at: parse id", ex);
}
You've gone out your way to destroy the information being passed up when using exceptions, to demonstrate the value in having error types.
It would be far more conventional to also provide a `TryDeactivateUser` function that cannot throw an exception. The article does note this, but hand-waves it away.
I'm not against Result types, but I don't find this article entirely convincing.
Replies
The original exception is available[1] in the InnerException though, so upstream can handle those differently.
[1]: https://learn.microsoft.com/en-us/dotnet/api/system.invalido...
This code path leaves entire classes of unhandled parse exceptions on the table. I have a hard time believing this could be intentional. The safest and most concise approach is to use int.TryParse on the inputId and throw if it returns false.
That's not how you're supposed to handle this kind of errors (according to .NET designers) - there's 2 kinds of errors in concept, everyday errors that are to be expected, such as a dictionary not containing a key, or in this case, a user supplying a badly formatted integer. For this you have the Try.. methods with TryGetValue, TryParse etc.
Go for example, allows for multiple return values, so it allows more elegant handling of this exact cass.
Then there's the serious kind of error, when something you didn't expect goes wrong. That's what exceptions are for. If this distinction is followed, then you don't want to handle specific exceptions (with very few notable distinctions, like TaskCanceledException), you just either pick a recoverable function scope (like a HTTP handler), and let the exception bubble to its top, at which point you report an error to the user, and log what happened.
If such a thing is not possible, just let the program crash.
You're leaking implementation details if you let exceptions bubble. Sometimes this is ok if all of the callers are aware of the implementation details anyway, but it can make refactoring or changing implementations more difficult otherwise.
Doing it like this (or preferably with a custom exception) translates the technical problem into a domain problem. Without doing this, callers can't properly handle it. FormatException or OverflowException could be thrown at multiple locations, not just in parsing the user ID. This here is an InvalidUserIdException. It could be derived from ArgumentException, but IMHO InvalidOperationException is not appropriate.