HiveBrain v1.2.0
Get Started
← Back to all entries
principlecsharpMinor

C# approach to prevent null references in strings

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
preventnullstringsreferencesapproach

Problem

My insight here is that it's usually not useful to distinguish between null and empty strings. Sometimes it is, but usually not. Usually I like to assign my strings as empty if they would have been null.

var lastName = Request.Params["lastName"] ?? ""

Thus I had the idea of a struct which would wrap a real string, internally coalesce it to "", prevent all manner of null references, provide "less dangerous" versions of common string operations, yet still be compatible with strings.

This is an incomplete listing--I just wanted to get feedback on the idea. Please let me know why this is awesome, mediocre, or just plain terrible!

//with normal string
public void WriteName(string name)
{
    if(name != null && name.Length > 30)
    { name = name.Substring(0,30); }

    Console.WriteLine(name);
}


//with safestring
public void WriteName(SafeString name)
{
    Console.WriteLine(name.ReduceTo(30));
}


Below is the SafeString struct:

``
///
/// Struct wrapper for a string, to prevent null references when you
/// don't care to distinguish between
null and empty strings.
/// Has implicit conversions to & from string
///
public struct SafeString : IEquatable, IEquatable
{
/*
The wrapped string value.
This will be properly initialized only if the constructor is called.
It's possible to create a struct with
default(SafeString)
or by calling the parameterless ctor,
new SafeString()
C#6 will allow us to define a parameterless ctor,
but there is still the issue of
default(SafeString) which does not call any ctor.
Thus when we wish to use the value of
_theString`,
we need to either to check for null or funnel access through a helper
*/
private string _theString;

public SafeString(string s)
{
_theString = s ?? "";
}

public override string ToString()
{
//perform initialization in case this was created without using the ctor

Solution

System.String is a class, a reference type. Wrapping it in a value type makes for a pretty confusing start: it's like the opposite of boxing, where a value type is wrapped with a reference type. It's ...awkward.

Value types should be immutable; _theString doesn't immediately strike me as a readonly field:

private string _theString;


And _theString is a very, very bad name - I would have simply gone with _value, like this:

private readonly string _value;


Calling ToString shouldn't have side-effects - here calling ToString is mutating the instance, changing the reference stored in _theString:

public override string ToString()
{
    //perform initialization in case this was created without using the ctor
    return _theString ?? (_theString = "");
}


There is little to no gain in doing this, and it makes your value type violate a fundamental rule: value types should be immutable, no ifs, no buts. I'd change it to this:

public override string ToString()
{
    return _value ?? string.Empty;
}


The only way client code can retrieve the encapsulated value, is by calling ToString - this is practical and useful, but not exactly instinctive. I would expose a getter:

public string Value { get { return ToString(); } }


The Equals(string) implementation relies on implementation details of another method:

return ToString() == "" //we are never 'null'


I don't see a need for an explanatory comment when using "normal" string methods:

return ToString().IsNullOrEmpty();


You have hard-coded "" in a bunch of places - I find string.Empty conveys "" in a more unambiguous way, and I would replace all occurrences of "" with string.Empty.

I think your struct is violating the Single Responsibility Principle, with methods like ReduceTo - I think these members belong as extension methods, extending the String type itself.

Also when writing code at that level, you should make sure every public member has proper XML comments, so that client code consuming it has a bit of documentation in IntelliSense.

Code Snippets

private string _theString;
private readonly string _value;
public override string ToString()
{
    //perform initialization in case this was created without using the ctor
    return _theString ?? (_theString = "");
}
public override string ToString()
{
    return _value ?? string.Empty;
}
public string Value { get { return ToString(); } }

Context

StackExchange Code Review Q#77430, answer score: 5

Revisions (0)

No revisions yet.