I love guard clauses and conditions. Simply put, a “guard” in programming is something that checks that inputs are as expected. If they are not, the code either fails or defaults to some predefined result. I want to argue that these are an essential tool when writing code, even by yourself. Guards provide safety and reduce debugging time. When used correctly, they improve logic flow and enhance readability.
Fatal guard clauses
The canonical guard clauses that most programmers have seen and experienced are method argument exceptions:
|
1 2 3 4 5 6 |
public void AddHealth(int amount) { if (amount <= 0) throw new ArgumentOutOfRangeException(nameof(amount)); Health += amount; } |
The code simply expects value(s) to fall within a certain range. If not, the code throws an exception. These should ideally never happen in production code.
I want guards all over the code. I want unexpected inputs to trigger immediate failures, because it is pretty much always my fault. I don’t want the code to keep running and fail at some later time when I can no longer easily trace back the original issue (for example, a null stored in a collection could remain unused and undetected until much later).
More importantly, the things I check may not be directly relevant to the code I will run, for example, checking some data key conflicts:
|
1 2 3 4 5 6 7 8 9 10 |
public void SetInt([NotNull] string key, int value) { if (key == null) throw new ArgumentNullException(nameof(key)); if (key == "" || reservedKeys.Contains(key)) throw new ArgumentOutOfRangeException(nameof(key)); if (ints.ContainsKey(key)) throw new ArgumentOutOfRangeException(nameof(key)); if (bools.ContainsKey(key)) throw new ArgumentOutOfRangeException(nameof(key)); if (strings.ContainsKey(key)) throw new ArgumentOutOfRangeException(nameof(key)); ints.Add(key, value); } |
The code in this real example would happily run and the game would play fine until I tried to load a save and then it would fail silently to restore some value because one of the keys would not be loaded. And this is the worst-case scenario bug: a silent one that causes no alarms.
Non-fatal guard clauses
Non-fatal guard clauses are part of “your own code” that return early if they fail, but do not cause exceptions or halt the code. This is where some programmers might draw the line, because such code encroaches into business logic. And I agree — this is what I might call “decision logic” prior to “actuation logic”. I will continue this post with the understanding that we accept that our style and design permit these. Technically, these are not “true” guards anymore, but I have no better name for these.
A simple example like this:
|
1 2 3 4 5 6 7 |
public void TryJump() { if (CurrentState != ActionState.Idle) return; Jump(); } |
A more complex example would have a return result:
|
1 2 3 4 5 6 7 8 9 |
public bool AddCoins(int amount) { if (Coins + amount > MaxCoins) return false; Coins += amount; CoinsAdded?.Invoke(); return true; } |
This is like combining a CanAddCoins() check and DoAddCoins() action (that throws an exception if it cannot).
These are very simple examples and the same code could be written with a direct conditional check:
|
1 2 3 4 5 6 7 8 9 10 11 |
public bool AddCoins(int amount) { if (Coins + amount <= MaxCoins) { Coins += amount; CoinsAdded?.Invoke(); return true; } return false; } |
In fact, I’ve written plenty of these and there’s nothing inherently wrong about it. Where the trouble starts is when more of these conditions are added and start piling up:
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 |
public bool AddCoins(int amount) { if (amount >= 0) { if (Coins + amount <= MaxCoins) { if (WalletActive) { if (amount > 0) { Coins += amount; CoinsAdded?.Invoke(); } return true; } } } return false; } |
In contrast to adding new guard clauses, here we quickly descend into many nested statements until we lose readability. This is why I prefer to write even the simple cases in the form of guard clauses — self-imposed consistency, readability and future-laziness proofing.
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 |
public bool AddCoins(int amount) { if (!WalletActive) return false; if (amount < 0) return false; if (Coins + amount > MaxCoins) return false; if (amount == 0) return true; Coins += amount; CoinsAdded?.Invoke(); return true; } |
The reason I call this more readable is because you can read each guard clause as a self-contained “sentence” with explicit return result. They are easy to reorder, expand, suspend and remove. The business part of the method is separate† from the guarding part. Even unit test writing becomes simpler when one can compare degenerate test cases with guard clauses.
†As I mentioned at the start, one can argue these are part of the business logic. And I do not disagree. The distinction here is between deciding to and acting upon the business rules. Guard clauses do not actuate anything. Final business code does not decide whether to actuate. Which might seem like an arbitrary hair-splitting until one writes and maintains a thousand of these methods.
Inline guard clauses
If anyone was offended by my use of non-guard guard clauses in the previous section, do I have a section for you! This is where many programmers might draw the line, even if everyone agrees nesting if statements is bad and having else should avoided when possible.
What I call “inline” guards are non-fatal guard clauses that appear in the middle of other code. Guard clauses are generally expected at the start of the method. But occasionally it is useful to place them inline as a way of writing and organizing code and nested statements:
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 |
private int CountConsumableItems(ItemType type) { int total = 0; foreach (Item item in Items) { if (!item.consumable) continue; if (item.type != type) continue; total += item.amount; if (item.stackable) total += item.stacks * 64; } return total; } |
I would like to argue that I am using guard clauses as intended here at the start of each loop. Each loop concerns itself with a separate scope and entity and thus can be thought of as its own separate method. In fact, I would argue that what I really wrote is this:
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 |
public int CountConsumableItems(ItemType type) { int sum = 0; foreach (Item item in Items) sum += GetConsumableAmountOfItem(item); return sum; } private int GetConsumableAmountOfItem(Item item) { if (!item.consumable) return 0; if (item.type != type) return 0; int total = item.amount; if (item.stackable) total += item.stacks * 64; return total; } |
I don’t think splitting methods is more readable and necessary just for the sake of clean-code method-separation purity. If one is familiar with guard clauses, they understand where they appear in the code and can recognize where the code has boundaries, such as the inner loop content above. In my opinion, it is perfectly fine that a small method might sacrifice some design purities for compactness.
The main difference I want to highlight again is that I am not doing nested statements:
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 |
private int CountConsumableItems(ItemType type) { int total = 0; foreach (Item item in Items) { if (!item.consumable) { if (item.type != type) { total += item.amount; if (item.stackable) total += item.stacks * 64; } } } return total; } |
This is a simple example, but even here we’re 3 levels deep and not yet done with conditions. Real code will have even more conditions and more checks, and plenty other stuff. I often write long, complex methods with multiple inner guards:
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 |
foreach (StylingEntry style in styles) { foreach (Area area in areas) { if (!DoesAreaFitStyle(area, style)) continue; foreach (Node node in area.nodes) { if (!DoesNodeFitLocations(node, locations)) continue; ... } } } |
There are good arguments why this is a code smell and why one should avoid inner loops to begin with, split them into method calls, etc. But, at some point, I have to write this code before I can potentially refactor it. And one of the ways to make it easier to refactor is when I can clearly see my guard conditions. I have seen what happens when condition-fuelled code is piled on top of code to the point where no one understands it anymore or could refactor it. Programmers become afraid to unroll the nested conditions and make bugs trying to preserve jump statements. Incidentally, this is where refactoring tools like ReSharper shine with their ability to do common refactors like inverting if statements.
Conditional compilation
99% of guards are useless to me in production code, because I cannot handle exceptions gracefully, such as in a typical video game. I work with Unity a lot and I often make complex guard clauses, especially for things I specify via Unity’s inspectors, which–in my experience–are very prone to human error. And these are often expensive checks that are unsuitable for tight game loop timing. So I wrap these up in #if UNITY_EDITOR pre-processor condition (similar to just #if DEBUG in “normal” C# project):
|
1 2 3 4 5 6 7 8 9 |
public Entry GetAnimationSpeed(AnimationType animationType) { #if UNITY_EDITOR if (!Enum.IsDefined(typeof(AnimationType), animationType)) throw new InvalidEnumArgumentException(nameof(animationType), (int)animationType, typeof(AnimationType)); if (entries.All(e => e.type != animationType)) throw new ArgumentNullException(nameof(animationType)); #endif return animations.First(e => e.type == animationType).speed; } |
Here I’m checking that I haven’t left a bad enum value or forgotten an entry entirely in the inspector. Granted, even without the guards, this code would just fail on the next line, but I want it to fail and tell me exactly what value caused it rather a generic unhelpful exception.
Note that one can use debug-only conditionally-compiled Debug.Assert() or Unity’s own UnityEngine.Assert() instead. These are just fancier and less recognizable ways of accomplishing the same thing. However, I find these slightly less readable. And, when working with Unity, I actually want the game to not have assertions when I make a stand-alone build, even when it is compiled as a development build, in other words, with the DEBUG conditional.
I also often place fatal and non-fatal conditional guard clauses in the middle of code:
|
1 2 3 |
#if UNITY_EDITOR || DEBUG if (selectedProps.Count == 0) throw new Exception("No props provided by prop selection ... "); #endif |
These are a very useful debugging tool. If the code works as expected, I can forget that I ever added these. If the code fails on these, I will have a clear indication where and why it happened, especially many months if not years into the future. I am okay with placing these wherever in the code, because they are clearly marked and because the benefits of catching intention failure cases in complex logic is worth the loss of code prettiness.
Code annotations
My favourite code-sugar feature is adding annotations, such as [NotNull] :
|
1 2 3 4 5 6 |
public bool AddItem([NotNull] Item item) { if (item == null) throw new ArgumentNullException(nameof(item)); StoredItems.Add(item); } |
ReSharper does this automatically for common argument checks. These static contracts provides helpful hints for the IDE and can guard (har-har) against human errors. These can get pretty extensive (and I love it!):
|
1 2 3 4 |
[Pure, NotNull, ItemNotNull] public List<string> GetStringList([NotNull] string key) { ... |
Bad guarding
I added quiet a bit of elaboration and notes in the previous sections regarding the “purity” of using guard clauses, especially mixed with other code. (I will not go into how one could refactor code to not even need guard clauses — for most practical purposes (and certainly, mine), that will remain a mental exercise.) There are some issues worth mentioning, although you will find that the common theme for these is that they are actually a symptom of a different problem.
Many consecutive inline guard clauses usually indicate that the method just knows too much and is too big and it’s time to split it into smaller or different ones. And it is likely a class should be extracted instead (which is a whole another topic).
Guard clauses mixed between business logic (check, act, check, act, etc.) are an indication that the method has long failed its single responsibility and, again, needs splitting and likely being into extracted into a class.
Over-reliance on guard clause leads to over-reliance on jump statements: return , continue , break. And let’s be honest: these are just goto statements in disguise. They are, of course, better because they each have one meaning. But they are still statements that hinder reading code by making you “jump” elsewhere. Again, it is likely that methods needs splitting and encapsulating. Likely, a different approach should be used and data pre-processed in steps (which is yet another topic).
On the opposite end, if a class has many methods and no guard clauses in sight, this likely indicates that the behaviour is undefined with exceptional inputs. Unless the class is fully unit tested, because then the unit tests act as the guards (but this too is a whole another topic).
All in all, as I mentioned, well-written guard statements are easy to modify. These problems are not caused by the guards themselves, but by the surrounding code. If anything, having clear guard clauses slightly mitigates these problems. One of the steps in refactoring towards cleaner code will likely involve separating guard clauses from the business just to understand what is happening.
Final comments
In conclusion, I want to say that guard clauses work very well for me. Other might (reasonably) argue that it is not worth their effort. My particular needs and environment lend itself to such design that I can justify the investment of any upfront time I spend on these. But do note that I primarily work on video games and the codebases are large, intertwined, and constantly evolving while delivery times are short and specification varies from loose to non-existent.