AArnott / Validation

Method input validation and runtime checks that report errors or throw exceptions when failures are detected.
Microsoft Public License
131 stars 22 forks source link

Additional overloads for Requires methods #63

Closed scottdorman closed 3 years ago

scottdorman commented 3 years ago

Some of the Requires methods, notably NotNullOrWhiteSpace and NotNullOrEmpty, would be helpful to include another overload (or maybe set of overloads) which allow a message parameter. This would allow the calling site to customize the message used in the exception.

AArnott commented 3 years ago

It's curious that you'd need to customize it. How would a custom message make it easier to understand what they did wrong? Note that instructing the caller as to what they should be passing in instead of null/empty/whitespace IMO is outside the scope of the exception, IMO. Folks should get exceptions when they do something wrong -- not to find out how to do it right. xml doc comments should answer that.

scottdorman commented 3 years ago

The specific use case is this:

Given the following object model, which represents a strongly-typed configuration object populated from a JSON configuration file (using the standard .NET configuration system)

public class AuthConfiguration
{
   public string? RedirectPath { get; set; }
   public IEnumerable<string>? Scopes { get; set; }
}

I want to validate the configuration at startup using a class similar to

internal static class AuthConfigurationValidator
{
   public static void Validate(AuthConfiguration configuration)
   {
      Requires.NotNullOrWhiteSpace(configuration.RedirectPath, nameof(configuration.RedirectPath));
      Requires.NotNullOrEmpty(configuration.Scopes, nameof(configuration.Scopes));
   }
}

In this context, it is a better error message to say "Redirect path configuration parameter is missing." or "Scopes configuration parameter is missing or empty." rather than one of several possible messages (for the simple string property: "cannot be null", "cannot be an empty string ("") or start with the null character.", or "The argument cannot consist entirely of white space characters."; for the string enumerable property: "cannot be null" or "must contain at least one element.").

AArnott commented 3 years ago

Your Validate method shouldn't be throwing ArgumentNullException IMO because the argument isn't null (or at least, you're assuming it isn't as you don't even check it). Instead, ArgumentException is most appropriate since it's a specific complaint against the argument that isn't that the argument itself is null. And the ArgumentException throwing arguments do allow you to set the message.