Note: This post is part of the SciFiDevCon 2023: May the Fourth Be With You event. Ardalis and Sadukie happen to be a part of the Stir Trek planning committee, and this year’s movie is Guardians of the Galaxy 3. So in this post, Sadukie is running with a Guardians of the Galaxy adventure in code.
The Guardians of the Galaxy have written some code for an app to keep track of their adventures. Star-Lord and Gamora have been pairing on the code, though both of them are fairly new programmers. They know that Rocket has been trying to impersonate Groot in the entries, but Rocket is not that good at impersonating Groot. Knowing that we’re here to help software devs become better developers, the Guardians reached out to us to help them with their code. In particular, they have a C# method that does validation that needs some refactoring. So let’s take a look at their guard clauses and see how we can help them out. This is their code, edited for brevity:
protected void SaveEntry(string author, string message)
{
if (string.IsNullOrWhiteSpace(author))
{
throw new ArgumentException("Invalid author");
}
else
{
if (string.IsNullOrWhiteSpace(message))
{
throw new ArgumentException("Invalid message");
}
else
{
if (author.ToLower() == "groot")
{
if (message.ToLower()!="i am groot")
{
throw new ArgumentException("Fake Groot detected!");
}
else
{
// Save logic here
}
}
else
{
// Save logic here
}
}
}
}
Code Smells
Looking at the code, there are a few things that really stand out.
Repetitive Code
Their save logic was repeated throughout the method. We recommended the save logic get moved to its own method. One of the principles we taught the Guardians was DRY - Don’t Repeat Yourself.
Bumpy Road Code Smell
The Bumpy Road code smell is seen in the shape of the code. There are many layers of indentation, which slow down the code comprehension. Much like a bumpy road slows a drive, the code in this bumpy road pattern slows our understanding of what’s going on. When you see layers upon layers of nested conditional logic, remember that it can probably be written in a more readable way.
Unnecessary Code
There are also many else
clauses that aren’t needed based on the code around it and an if
expression that can be combined. This is what their code looked like after removing the unnecessary else
clauses.
protected void SaveEntry(string author, string message)
{
if (string.IsNullOrWhiteSpace(author))
{
throw new ArgumentException("Invalid author");
}
if (string.IsNullOrWhiteSpace(message))
{
throw new ArgumentException("Invalid message");
}
if (author.ToLower() == "groot" && message.ToLower()!="i am groot")
{
throw new ArgumentException("Fake Groot detected!");
}
// Save logic here
}
Notice that by removing the unnecessary else
statements, the bumpy road is smoothed out.
One-Line Expressions (Optional)
With this being C#, the language supports one-line if
statements. While there may be thoughts of “What if we have to add more logic for validating our inputs?”, we taught the Guardians about the principles of YAGNI - You Aren’t Gonna Need It and keeping it simple.
We can simplify their code further:
protected void SaveEntry(string author, string message)
{
if (string.IsNullOrWhiteSpace(author)) throw new ArgumentException("Invalid author");
if (string.IsNullOrWhiteSpace(message)) throw new ArgumentException("Invalid message");
if (author.ToLower() == "groot" && message.ToLower()!="i am groot") throw new ArgumentException("Fake Groot detected!");
// Save logic here
}
This is significantly more compact compared to their original method. However, is there more we can do here? We can deal with the repetitive null checks using Ardalis.GuardClauses
.
Guard Clauses
Ardalis created a NuGet package called Ardalis.GuardClauses that makes it easier for us to capture that null check pattern. Once we add the package to our list of NuGet packages in the project, we are able to simplify the null checks.
Built-in Guard Clauses
Looking at the GitHub repo for GuardClauses, the supported guard clauses include:
- Null
- Null or empty
- Null or whitespace
- Out of range
- Enum out of range
- Out of SQL date range
- Numeric guard clause for 0
Since there is support for null or whitespace, this is what the code looks like when we implement the supported guard clauses:
using Ardalis.GuardClauses;
// additional logic
protected void SaveEntry(string author, string message)
{
Guard.Against.NullOrWhiteSpace(author,nameof(author));
Guard.Against.NullOrWhiteSpace(message,nameof(message));
if (author.ToLower() == "groot" && message.ToLower()!="i am groot") throw new ArgumentException("Fake Groot detected!");
// Save logic here
}
Custom Guard Clauses
Looking at this logic, there is still a guard clause to check for those trying badly to be Groot. We can create a custom guard clause with Ardalis.GuardClauses
to encapsulate this logic.
-
Create a static class that is in the
Ardalis.GuardClauses
namespace.namespace Ardalis.GuardClauses; public static class FakeGrootGuard { }
-
Add a static extension method that takes
IGuardClause
as the first parameter. Then, include additional parameters as needed. Since we are checking author and message, we will pass those in:namespace Ardalis.GuardClauses; public static class FakeGrootGuard { public static void FakeGroot(this IGuardClause guardClause, string author, string message) { if (author.ToLower() == "groot" && message.ToLower() != "i am groot") throw new ArgumentException("Fake Groot detected!"); } }
The validation logic with the custom guard clause will look like this:
protected void SaveEntry(string author, string message)
{
Guard.Against.NullOrWhiteSpace(author,nameof(author));
Guard.Against.NullOrWhiteSpace(message,nameof(message));
Guard.Against.FakeGroot(message,author);
// Save logic here
}
This is more readable and more compact compared to where they started.
Extra Credit: FakeGrootException
Groot is legendary enough to deserve his own exception. So let’s get him his own exception like this:
public class FakeGrootException : Exception
{
public FakeGrootException() : base("You are not Groot!") {}
}
Our FakeGrootGuard
can throw the new exception with the following code:
namespace Ardalis.GuardClauses;
public static class FakeGrootGuard
{
public static void FakeGroot(this IGuardClause guardClause, string author, string message)
{
if (author.ToLower() == "groot" && message.ToLower() != "i am groot") throw new FakeGrootException();
}
}
Conclusion
When looking at validation logic, some code smells to watch out for include:
- the bumpy road pattern
- repetitive logic
- unnecessary
else
- separated
if
statements - “just in case”
Once you eliminate some of those smells, the bumpy road becomes smoother. Adding Ardalis.GuardClauses can reduce some of the repetitive null checks and other repetitive guard patterns, and it is extensible for you to add your own patterns.
As for the Guardians, once they implemented these changes, their code was more maintainable for Star-Lord and Gamora. Rocket was no longer able to try to impersonate Groot.