Exceptions to control execution flow in managed .NET code - a good practice or not?

Question

Here is a very simple functionality example, to make the discussion easy. Suppose, I am logging some abstract objects' content in a flat log. If an object's json representation contains property called EntityID with a GUID, I'd like to record it as a separate data element, if not - no matter, we still store the json but without the additional metadata.

So, I'll need a method which would return an ID if we are able to extract it or null if not. My instincts tell me to code it like this:

    private Guid? ParseSyncTableUniqueIdFromContent(string content)
    {
        try
        {
            object contentObject = JsonConvert.DeserializeObject(content);
            if (contentObject is JObject typedContentObject)
            {
                return Guid.Parse(typedContentObject.Value<string>("EntityID"));
            }
        }
        catch
        {
            // well, ok then
        }

        return null;
    }

The code in try section might fail in a number of ways: content might be null, it might be not null but cause some other exceptions inside DeserializeObject, and then even if we manage to deserialize, the object might not contain the property and even if it does it might not be parseable into a GUID. Since I don't care which problem would prevent me from getting to the value, I simply catch anything that might happen and move on.

An alternative would be to introduce several validations which would just return a null immediately.

if (string.IsNullOrEmpty(content))
    return null;

etc.

My sense of aesthetics and my habits say that a catch-all in this case is acceptable. But now let's remember that we are inside the managed .Net code and that makes me wonder. Does the exception mechanism become a resourse drain, then? How significant is the overhead on handling the exception in .Net?

The reason I ask is actually this. This particular function is quite often called within a large, high-load application, which employs an (in my opinion not well justified) method of catching all of the exceptions in the FirstChanceException event and tracking them all in Application Insights (I do think it's too blunt, but that's a different story for a different time, I guess). But it makes me think of such use of exceptions in .Net - is it too "expensive" even without such detailed logging taken into account? Is it (much) more efficient to write validation checks? (IMO, would suck if it is like that, but I feel like I need to know)

no. bad idea. exceptions are _quite_ expensive. and exceptions should, as the name implied, be used in _exceptional circumstances_. regular control flow is, by definition, not _exceptional_ — Franz Gleichmann, Jun 28 '22 at 09:18
Does this answer your question? [Why not use exceptions as regular flow of control?](https://stackoverflow.com/questions/729379/why-not-use-exceptions-as-regular-flow-of-control) (search term: "exception control flow") — Franz Gleichmann, Jun 28 '22 at 09:19

Heinzi · Answer 1 · 2022-06-28T10:15:18.327

Aside from the general recommendation of not using exceptions for control flow, the bigger problem of your code is that your error handler is much too general.

Your server ran out of hard disk space? -> Your method returns null.
You made a mistake when coding that causes an exception in rare edge cases? -> Your method returns null.
Anything else you didn't think of? -> Your method returns null.

In these cases, you want .NET to throw an exception and tell you about the problem - the more details, the better! Otherwise, you hide the real cause of the problem and start scratching your head why, suddenly, just on the customer's system, your method started returning null for perfectly valid input.

Thus, if you decide to violate the general rule and use exception handling for regular control flow (for which there can be valid reasons, but it should be a deliberate decision), at least make your exception handler catch as little as possible. In your case, that would be the specific exception thrown by JsonConvert.DeserializeObject when getting invalid input, and, in a separate try-catch block, the FormatException of Guid.Parse. (Better yet, replace the latter by Guid.TryParse, which is available in .NET Framework 4 and newer.)

True. However, if I choose to only catch a selection of exceptions that are known cases here, say, JsonReaderException, ArgumentNullException, ArgumentException (whatever else that would "normally" just indicate missing data), the original question remains. Would validations be better? — Yuri Makassiouk, Jun 28 '22 at 09:34
@YuriMakassiouk: The general rule (see the link in my question for details) is to prefer validation over exception handling, unless you have good reason to deviate from that rule. If you only catch selected exceptions (and only in the lines where you expect them), you will need multiple try-catch blocks, so the "good reason" you had for using exception handling over validation (less code) no longer applies. Of course, there might be other good reasons for exceptions over validation (for example, the lack of a `JsonConvert.TryDeserialize` method) for specific lines in your code. — Heinzi, Jun 28 '22 at 09:55

Exceptions to control execution flow in managed .NET code - a good practice or not?

1 Answers1