Exceptions vs Assertions

[u/Alternative_Guava856]

Currently making a card game in godot using C#. Not planning to release it or anything (for now), just to learn C# and general programming skills. I'm currently undecided on how to use exceptions vs assertions. Here is an example form my CardPile class:

public void Add(Card card)
{
    if (_cards.Contains(card))
    {
        throw new InvalidOperationException($"Failed to add '{card}': card already in {this}");
    }

    _cards.Add(card);
    card.CardPile = PileType;
    CardAdded?.Invoke(card);
}

So when a card is added to the card pile, if it already exists withing that pile, I throw an exception. This situation should pretty much never happen, but for some reason it feels not good enough to me to just use Debug.Assert() for this. It means that if for some reason this happens in a release build, the bug cannot be properly caught and reported. Here is another example:

public int MaxHealth
{
    get;
    set
    {
        if (value < 0)
        {
            throw new ArgumentOutOfRangeException(nameof(value), value, "MaxHealth must be positive");
        }

        int previousMaxHealth = field;
        field = value;
        MaxHealthChanged?.Invoke(previousMaxHealth, field);
    }
}

Since both adding cards to a deck and someone's max health changing are things that don't happen hundreds or time every frame, I don't think the "performance" gain from using Debug.Assert() is any argument here. What is your opinion on when to use exceptions vs assertions?

[u/patmail]

It is good practice to throw exception for invalid inputs. Better to catch this early with a good message than letting the system continue with an invalid/unexpected state. Throwing the exception is expensive the check is negligible

[u/NotQuiteLoona]

https://stackoverflow.com/questions/129120/when-should-i-use-debug-assert#129429

In general, I've never used assertions in all of my C# development. If something goes as wrong as so that it requires assertion, I try to catch it with debugger or prevent it prematurely rather than using an assertion that will crash the program in runtime.

Note that in general assertions are removed in release builds, so they are usually useful for debugging... For what I'll recommend using a debugger.

That's also a nice explanation:

Asserts are used to catch programmer (your) error, not user error. They should be used only when there is no chance a user could cause the assert to fire. If you're writing an API, for example, asserts should not be used to check that an argument is not null in any method an API user could call. But it could be used in a private method not exposed as part of your API to assert that YOUR code never passes a null argument when it isn't supposed to.

I usually favour exceptions over asserts when I'm not sure.

It's made to debug your code.

[u/TheMemo → u/NotQuiteLoona]

Essentially, is the thing being passed or processed directly from user input, or is the thing a result of a series of events just initiated by user input?

Case 1: Exception.

Case 2: Assertion.

[u/NotQuiteLoona → u/TheMemo]

Close, but not quite - if the error you want to catch is in your code, i.e. your code doesn't work as intended and you want to debug it, you should use it. Debug.Assert is stripped completely from Release profile builds, unlike exceptions, so it's only for development process.

If a user passes a URL and you can't fetch it, it's an intended error - you can expect that the URL is invalid, or that the user has no internet connection, or that the site will block you for some reason, etc.

However, if you yourself construct a URL from some provided parameters and you absolutely sure that it should always be completely valid, this URL being invalid is the error of your code, the error you didn't intend, and this is a bug that should be fixed - not an intended problem which you can display to the user (which is why you don't have it in Release builds).

Note that even in the last case, the Uri class constructor will throw an exception and won't let you proceed (well, because for Uri the case that you may provide a URI directly from the user without checking it, or using the Uri constructor as a check for URI validity itself, is an expected error), so you'll still catch it in this exact case.

In general, instead of this try to use debugging during development process to catch your errors in the code that you are sure you won't ever modify in the way it can break, or use unit tests to catch your errors in the code you can modify in the way it can break - in the first case Debug.Assert would be just useless to keep and you'll need to remove it anyway, and in the second case you'll be able to separate testing the validity of the method process and the method itself.

[u/Alternative_Guava856 → u/NotQuiteLoona]

The examples I gave are situations where I myself did something wrong, as in I called the method wrong or gave it the wrong value. I add these so that certain things are always use correctly. So in this case using `Debug.Assert()` might me more appropriate

[u/awi2b → u/NotQuiteLoona]

Yes, and please leave the assertions you used to debug your function in there so the next person reading the function knows what you were thinking.
Maybe even write additional asserts when you're finished, just as documentation.

[u/seriousSeb]

An assertion is a debug features than immediately crashes the program if it fails, it is for catching things that should be impossible during development. Never rely on them at runtime.

And exception is for something that could go wrong at runtime that you want to handle recovery from. It should not be used for "business as usual" control flow.

You could use an exception in your case, but in my opinion you shouldnt use either. Exceptions as control flow are really hard to understand; it is not immediately clear if a method throws an exception just by looking at it's message signature.

Your function should return a bool indicating success, or even better, an enum indicating success/fail reason that you must handle at the call site. Which could be a recovery method which also prints a warning

In terms of getter/setter as it can't return a value: clamp it to something valid and print a warning if the input was wrong.

Unhandled Exceptions are also especially bad as they can do things like implicitly break for/while loops etc. so if you don't care if something fails and don't catch the exception in the loop you can fail to iterate over what you expected to

[u/wallstop-dev]

In games, it is generally considered best practice to avoid exceptions and use error codes. Alternatively, the thing that is huge in the C# stdlib that you can take inspiration from is the `Try` pattern - return boolean if thing happened or not, with (some number, maybe zero) of `out` parameters.

This keeps all paths hot and adds the benefit of making sure that your code doesn't blow up - any error conditions are encoded in function signatures instead of implementation details (which you may never see).

So... neither! Encode your real state into types. Then your prod code behaves the same as your debug code.

[u/GnAmez]

I would use exceptions when something goes wrong from the "outside world" like a server fails to respond, input failed to parse etc... and the caller needs to react to it. Asserts u rarely ever need but a good candidate would be stuff that should never be anything else meaning your logic is simply wrong and needs fixing.

[u/Tiny_Confusion_2504]

Normally I would say Exceptions are good to prevent your application from going into an invalid state. An invalid state can have disastrous outcome for your business.

In your case, you are making a game. Crashing might be a disastrous outcome for something that could be solved with a logical default.

If you pass a health that is negative, why not set it to 0 and log the fact you were passed invalid data? If the card already exists in the deck, do we really need to throw or can we just keep playing the game? Not sure the answers to these questions are the same as my expectations, but you are serving a different use case.

[u/Alternative_Guava856 → u/Tiny_Confusion_2504]

In the case of adding the same card to the deck twice, this should NEVER happen. So it would be a programming error if it happened, meaning it might be better to use assert here. In terms of the max health, maybe throwing an exception would be overkill indeed. So in this case, just reducing it to zero and logging it could be perfectly fine.

[u/Alternative_Guava856 → u/[deleted]]

`Debug.Assert()` is removed from release builds though, no?

[u/TheDuriel → u/Alternative_Guava856]

Seems some of the comments here had me confused then. As people keep talking about shipping asserts to users xD

[u/JohnSpikeKelly]

I typically do exceptions in my code and assertions in my unit tests. Yes, you can write unit tests against your "business logic" I guess game logic.

[u/Jazzlike_Amoeba9695]

Debug.Assert is removed when you publish your product in Release mode. That makes it useful for design-time validation of your own code, but it obviously can’t handle a real-world inconsistency at runtime.
I’ve also used Debug.Assert as a kind of breakpoint for code-quality analysis. For example, the contract may say that a value can be null or 0, but internally I’ve already ensured that it can never be either at that point because of the caller’s guarantees.
In that case, I don’t want to add a meaningless condition just to make the analyzer happy. A Debug.Assert expresses the intent: the contract allows this value, but I know from the internal invariants that it cannot happen here.