6
\$\begingroup\$

I've written a C# struct in order to encapsulate the idea of a string being neither null nor white space.

I was basically tired of writing and unit testing checks like the following:

public class Person 
{
   public string Name { get; }

   public Person(string name)
   {
      if (string.IsNullOrWhiteSpace(name))
      {
         throw new ArgumentException(
           "A person name cannot be null or white space", 
            nameof(name)
         );
      }

      this.Name = name;
   }
}

The idea is creating a type, let's call it NonEmptyString which is auto safe, so that I can use this type everywhere avoiding these annoying string checks.

I came up with the following (full source is available here):

using System;

namespace Deltatre.Utils.Types
{
  /// <summary>
  /// This type wraps a string which is guaranteed to be neither null nor white space
  /// </summary>
  public struct NonEmptyString
  {
    /// <summary>
    /// Implicit conversion from <see cref="NonEmptyString"/> to <see cref="string"/>
    /// </summary>
    /// <param name="nonEmptyString">The instance of <see cref="NonEmptyString"/> to be converted</param>
    public static implicit operator string(NonEmptyString nonEmptyString)
    {
      return nonEmptyString.Value;
    }

    /// <summary>
    /// Explicit conversion from <see cref="string"/> to <see cref="NonEmptyString"/>
    /// </summary>
    /// <param name="value">The instance of <see cref="string"/> to be converted</param>
    /// <exception cref="InvalidCastException">Throws <see cref="InvalidCastException"/> when <paramref name="value"/> is null or white space</exception>
    public static explicit operator NonEmptyString(string value)
    {
      try
      {
        return new NonEmptyString(value);
      }
      catch (ArgumentException ex)
      {
        throw new InvalidCastException($"Unable to convert the provided string to {typeof(NonEmptyString).Name}", ex);
      }
    }

    /// <summary>
    /// Creates new instance of <see cref="NonEmptyString"/>
    /// </summary>
    /// <param name="value">The string to be wrapped</param>
    /// <exception cref="ArgumentException">Throws <see cref="ArgumentException"/> when parameter <paramref name="value"/> is null or white space</exception>
    public NonEmptyString(string value)
    {
      if (string.IsNullOrWhiteSpace(value))
        throw new ArgumentException($"Parameter {nameof(value)} cannot be null or white space", nameof(value));

      this.Value = value;
    }

    /// <summary>
    /// Gets the wrapped string
    /// </summary>
    public string Value { get; }

    /// <summary>Indicates whether this instance and a specified object are equal.</summary>
    /// <param name="obj">The object to compare with the current instance. </param>
    /// <returns>
    ///     <see langword="true" /> if <paramref name="obj" /> and this instance are the same type and represent the same value; otherwise, <see langword="false" />. </returns>
    public override bool Equals(object obj)
    {
      if (!(obj is NonEmptyString))
      {
        return false;
      }

      var other = (NonEmptyString)obj;
      return this.Value == other.Value;
    }

    /// <summary>Returns the hash code for this instance.</summary>
    /// <returns>A 32-bit signed integer that is the hash code for this instance.</returns>
    public override int GetHashCode()
    {
      unchecked
      {
        int hash = 17;
        hash = (hash * 23) + (this.Value == null ? 0 : this.Value.GetHashCode());
        return hash;
      }
    }

    /// <summary>
    /// Compares two instances of <see cref="NonEmptyString"/> for equality
    /// </summary>
    /// <param name="left">An instance of <see cref="NonEmptyString"/></param>
    /// <param name="right">An instance of <see cref="NonEmptyString"/></param>
    /// <returns></returns>
    public static bool operator ==(NonEmptyString left, NonEmptyString right)
    {
      return left.Equals(right);
    }

    /// <summary>
    /// Compares two instances of <see cref="NonEmptyString"/> for inequality
    /// </summary>
    /// <param name="left">An instance of <see cref="NonEmptyString"/></param>
    /// <param name="right">An instance of <see cref="NonEmptyString"/></param>
    /// <returns></returns>
    public static bool operator !=(NonEmptyString left, NonEmptyString right)
    {
      return !(left == right);
    }
  }
}

Unfortunately in C# it is not possible to hide or editing the default constructor of a struct, so it is entirely possible to write the following code:

var myNonEmptyString = new NonEmptyString(); // default constructor is always available
string value = myNonEmptyString; // value is actually null

I thought of two ways of improving this type in order to handle this scenario:

  • use a default value, such as "N.A.", for the Value property. Doing so, even when an instance of NonEmptyString is created via the default constructor the wrapped string is actually a non empty string
  • add a private readonly field isInitialized, whose default value is false, in order to track whether the right constructor has been called (the field is set to true only in the constructor overload having the string parameter). Doing so it is possible to add a check at the beginning of each type member, so that an InvalidOperationException is raised each time the programmer creates an instance via the default constructor and tries to use it in his code.

Is there any other way to better handle the inevitable presence of the default constructor? What approach do you suggest?

For the ones asking themselves "why didn't he chose a class, in order to avoid this mess with the default constructor from the beginning", the reason for avoiding a class is simple: in C# (at least before C# 8) a reference type value is allowed to contain a null reference (and by default each reference type variable contains a null reference, unless properly initialized).

If NonEmptyString were be defined as a class it would be useless, because each piece of code receiving an instance of NonEmptyString would have to check whether the instance contains a null reference. Instead, I would like to get a type giving the guarantee that each possible instance contains an actual string (that is a string other than null, the empty string and a sequence of spaces).

\$\endgroup\$
  • \$\begingroup\$ @t3chb0t is this the same concept as your SoftString? \$\endgroup\$ – dfhwze Sep 20 '19 at 18:13
  • \$\begingroup\$ @dfhwze the values I want to avoid are null, string.Empty and strings like " " (I mean strings composed only of white spaces). The type models the idea of a string containing at least one character other than ' '. Intuitively these are the strings "having an actual value" \$\endgroup\$ – Enrico Massone Sep 20 '19 at 18:21
  • \$\begingroup\$ Ok now I see IsNullOrWhiteSpace. I don't think a default value makes sense. \$\endgroup\$ – dfhwze Sep 20 '19 at 18:23
  • \$\begingroup\$ @dfhwze me too, my idea is going with the second solution I proposed (the runtime check over the isInitialized flag). The point with the default value is that whichever default value you chose, it probably doesn't make any sense for the programmer using the type. It's not meaningful and it's just a trick to avoid a null. \$\endgroup\$ – Enrico Massone Sep 20 '19 at 18:29
  • 1
    \$\begingroup\$ @EnricoMassone I would go with a reference type (class), you can not get performance and safety the same time in this case. There should be a type without default constructor. I would define a base type like String<TValidator, TComparer> and inherit it in a situation like class ProductName : String<NotEmptyValidator, IgnoreCaseComparer> {}. I wish .NET had something like this ready to be used... \$\endgroup\$ – Dmitry Nogin Sep 20 '19 at 19:18
5
\$\begingroup\$
public static explicit operator NonEmptyString(string value)
{
  try
  {
    return new NonEmptyString(value);
  }
  catch (ArgumentException ex)
  {
    throw new InvalidCastException($"Unable to convert the provided string to {typeof(NonEmptyString).Name}", ex);
  }
}

The try/catch is superfluous, a guard clause should be throwing before the constructor is even invoked IMO; the exception could be an ArgumentNullException, but that would be confusing if value was actually string.Empty. Perhaps derive a custom exception from ArgumentException:

if (!string.IsNullOrEmpty(value))
{
    throw new NullOrEmptyStringArgumentException()
}

The problem is that it makes the if (!string.IsNullOrEmpty(value)) validation logic (and the conditional throw) show up in two places... unless we pulled it out of local scope:

private static void ThrowIfInvalid(string value)
{
    if(string.IsNullOrEmpty(value))
    {
        throw new NullOrEmptyStringArgumentException(...);
    }
}

So we get:

public static explicit operator NonEmptyString(string value)
{
    ThrowIfInvalid(value);
    return new NonEmptyString(value);
}

And the constructor can ThrowIfInvalid as well:

public NonEmptyString(string value)
{
  ThrowIfInvalid(value);
  this.Value = value;
}

Much simpler everywhere! Except... that doesn't solve the default constructor problem, and throwing in a getter violates CA1065. I would probably have these:

public static NonEmptyString Invalid { get; } = default;
public bool IsValid => this != Invalid;

The obj is NonEmptyString check in the Equals override works in non-obvious ways given a string argument, ...is the implicit cast involved? Would an explicit obj as NonEmptyString soft-cast seem more obviously correct here? Gotta love implicit operators! Let's refer to the docs!

User-defined conversions are not considered by the is and as operators.

https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/operators/user-defined-conversion-operators

So, it appears the implicit cast operator isn't involved: my understanding is that this means Equals is returning false if you give it a string, and I'd consider that a bug, given the implicit cast operator's intent is likely to make strings and non-empty strings seamlessly equate.

The ==, != operators should probably have an overload for string too, and + should be expected to work as well, and should even be expected to accept a NullOrEmpty string (and then you get += for free).

Could be just me, but depending on context I think I might prefer an extension method on string, over an implicit (or explicit) cast operator:

var foo = "hi".AsNonEmptyString();

The struct should probably also implement IEquatable and IComparable (+their generic counterparts, for string and NonEmptyString both), and IEnumerable+IEnumerable<char> too, for almost-complete parity with a string.

\$\endgroup\$
  • 3
    \$\begingroup\$ Ah, CA1065, got ya ;-) \$\endgroup\$ – dfhwze Sep 21 '19 at 20:26
3
\$\begingroup\$

Since an empty string is not allowed, I cannot see any good alternative for an Empty value. I would move the check to the getter of Value. This way, the exception is thrown on demand rather than on construction of an object. C# structs are required to have a default value Activator.CreateInstance(typeof(NotEmptyString));.

public string Value 
{
    get
    {
        if (string.IsNullOrWhiteSpace(value))
            throw new ArgumentException(
                $"Parameter {nameof(value)} cannot be null or white space", nameof(value));

         return value; // backing-field
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Throwing in a getter? \$\endgroup\$ – Mathieu Guindon Sep 21 '19 at 19:00
  • 1
    \$\begingroup\$ @MathieuGuindon oh boy, what do I have a feeling I'm violating something important here? :p \$\endgroup\$ – dfhwze Sep 21 '19 at 19:44
3
\$\begingroup\$

GitHub, NuGet

Let’s hope for a non-nullable references in the upcoming version of C#. As for now on I would say that the easiest way is just to check for null reference with ?? throw new ArgumentNullException().

Here is what I use to check for white spaces in my DTO/models - at the end of the day it allows to save on typing:

public class Dto 
{
    public Dto(ProductName name)
    {
        Name = name ?? throw new System.ArgumentNullException(nameof(name));
    }

    public ProductName Name { get; }
}

Json.NET will properly serialize read-only properties in both ways. My ProductName like classes are defined as:

public class ProductName : String<ProductName>
{
    public ProductName(string text)
        : base(text, NotNullOrWhitespace, Trim)
    {
    }
}

Where:

[JsonConverter(typeof(StringJsonConverter))]
public abstract class String<T> : ValueObject<T>
    where T: String<T>
{
    protected static string Trim(string text) => text?.Trim();
    protected static string EmptyIfNull(string text) => text ?? Empty;
    protected static string Upper(string text) => text?.ToUpper();
    protected static string Lower(string text) => text?.ToLower();

    protected static string NotNull(string text) => 
        text != null ? text : throw new ArgumentNullException(nameof(text));
    protected static string NotNullOrWhitespace(string text) => 
        !IsNullOrWhiteSpace(text) ? text : throw new ArgumentException("Text is required.", nameof(text));
    protected static string NotNullOrEmpty(string text) =>
        !IsNullOrEmpty(text) ? text : throw new ArgumentException("Text is required.", nameof(text));

    public static implicit operator string(String<T> s) => s?.Text;

    protected String(string text, params Func<string, string>[] actions) => 
        Text = actions.Aggregate(text, (acc, f) => f(acc));

    public string Text { get; set; }

    public override string ToString() => Text;

    protected override IEnumerable<object> EqualityCheckAttributes => 
        new[] { Text };
}

Where:

public abstract class ValueObject<T> : IEquatable<ValueObject<T>>
             where T : ValueObject<T>
{
    protected abstract IEnumerable<object> EqualityCheckAttributes { get; }

    public override int GetHashCode() =>
        EqualityCheckAttributes
            .Aggregate(0, (hash, a) => unchecked(hash * 31 + (a?.GetHashCode() ?? 0)));

    public override bool Equals(object obj) =>
        Equals(obj as ValueObject<T>);

    public virtual bool Equals(ValueObject<T> other) =>
        other != null &&
        GetType() == other.GetType() &&
        EqualityCheckAttributes.SequenceEqual(other.EqualityCheckAttributes);

    public static bool operator ==(ValueObject<T> left, ValueObject<T> right) =>
        Equals(left, right);

    public static bool operator !=(ValueObject<T> left, ValueObject<T> right) =>
        !Equals(left, right);
}

And:

class StringJsonConverter : JsonConverter
{
    public override bool CanConvert(Type objectType) =>
        objectType == typeof(object) ? false :
        objectType.IsConstructedGenericType && objectType.GetGenericTypeDefinition() == typeof(String<>) ? true :
        CanConvert(objectType.BaseType);

    public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer) =>
        Activator.CreateInstance(objectType, reader.Value);

    public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer) =>
        writer.WriteValue(value.ToString());
}

It is easy to extend the list of available operations:

public class Slug : String<Slug>
{
    protected static string Dash(string text) => text.Replace(" ", "-");
    public Slug(string text) 
        : base(text, NotNullOrWhitespace, Trim, Lower, Dash)
    {
    }
}
\$\endgroup\$
  • \$\begingroup\$ I'm wondering whether there is a cleaner way for handling ToUpper/Lower and make it case-insensitive without actually altering the original value. It would either require a copy of the input or an IEqualityComparer - probably a better choice. While Trim could be considered neutral, changing the case is an issue. \$\endgroup\$ – t3chb0t Oct 3 '19 at 18:49
  • \$\begingroup\$ I have one more idea. What do you think about making ValueObject<T> an IEnumerable<object> as a replacement for EqualityCheckAttributes? This could be useful in other scenarios too. \$\endgroup\$ – t3chb0t Oct 3 '19 at 18:53
2
\$\begingroup\$

Well, you could do what you want with some relatively "grey" magic, although I have not done this specific thing myself in the past. By the way, you still have to decide what code you will replace the default constructor with. If I understand well what you are trying to do, you would want the default constructor throwing an ArgumentException, is that right?

If you have some patience and a bit of spare time (not too much, really), the IL Assembler does not appear to have any problem with an explicit default parameterless constructor. As Guess Who* "successfully" (ok, no serious customization was performed anyway) poked around a long time ago, it appears to be feasible to do whatever you want inside an empty constructor for a custom type extending ValueType. You can use the article for a bit of guidance.

So, what I would try is:

  • Create an additional constructor in your almost-done type, receiving a dummy parameter (say, an int) and throwing an ArgumentException with the text informing that the the default empty constructor is not intended to be called directly (or any other exception you see fit).
  • "Bake" the almost-done type in a class library alone.
  • Disassemble the library back to CIL.
  • Read and understand a few basics of CIL so that I can...
  • ...remove the dummy int parameter from my constructor without causing other side effects in the code, so it would become the empty constructor.
  • Reassemble back using the ILAssembler directly from the disassembled, tampered IL code.

Then, boom, magically, I can never create an empty array of pre-initialized NonEmptyStrings anymore (for example NonEmptyString[] strings = new NonEmptyString[100]).

I assume this is grey area and you could feel better going with your bool solution anyway, but if you decide to give this a shot, I would very much like to know how this worked out.

*Also known as Jon Skeet!

\$\endgroup\$
  • \$\begingroup\$ +1 for an interesting approach. Not sure though if it works, I thought that default constructor is always being invoked from the custom one for all value types... \$\endgroup\$ – Dmitry Nogin Sep 22 '19 at 4:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.