6

This may well be well-known and discussed, but to my surprise I discovered today that you can give your own implementation of operators between nullables and their base types.

This means that you can have a struct which can be checked for null, and return true.

Now this would be convenient in my situation - on the advice of another member I have a struct that wraps strings. Being able to directly compare this wrapped string directly to null (rather than use a .IsNull or similar) is to me a lot more natural, and means changing from using strings to these WrappedStrings often require no other changes to the code.

But.. the fact that this feature is (I believe) little known, counter-intuitive to anyone thinking struct (which it is) over string (which it represents), and the fact that it confuses ReSharper (structValue == null warns "expression is always false") makes me think this is perhaps a dirty trick left in the dirty but neat tricks cupboard.

So I'm wondering, would you adopt it? If not, would you forgive me for doing so.. or is it really best not to go down this path?

public struct StringWrapper
{
    private readonly string str;
    public override string ToString() { return str; }

    public static bool operator ==(StringWrapper a, StringWrapper b) 
    { return a.str == b.str; }
    public static bool operator !=(StringWrapper a, StringWrapper b) 
    { return !(a == b); }
    public static bool operator ==(StringWrapper a, StringWrapper? b)
    {
        if (!b.HasValue || b.Value.str == null) return a.str == null;
        return a == (StringWrapper)b;
    }
    public static bool operator !=(StringWrapper a, StringWrapper? b) 
    { return !(a == b); }
    public static bool operator ==(StringWrapper? a, StringWrapper b) 
    { return b == a; }
    public static bool operator !=(StringWrapper? a, StringWrapper b) 
    { return !(a == b); }

    public StringWrapper(string str) { this.str = str; }
}
Community
  • 1
  • 1
Mania
  • 1,718
  • 14
  • 16
  • I think you are basically doing the same that the compiler does for `Nullable`. This is also a struct and it can also be checked for `null`. So... I guess, it is OK. However, I wouldn't do it. I can't propose an alternative, because I currently can't see the value your wrapper adds... – Daniel Hilgarth Sep 15 '11 at 06:28
  • 1
    `StringWrapper` in the real code represents an `InternedString` - that is a string where if it's the same value, it's guaranteed to be same instance. So `==` is implemented as a `object.ReferenceEquals()`, etc. Now it would be *nice* to be able to check an `InternedString` for null, but the default compare will always return "false", as a struct is never (normally) equal to (struct?)null. – Mania Sep 15 '11 at 06:32
  • And why do you need this? The string class already uses similar techniques. And why do you use a struct? Why don't you use a reference type? – Daniel Hilgarth Sep 15 '11 at 06:34
  • The string class does not intern by default. And if you choose to use string.Intern, you prevent that string from ever being collected and are granted no type-safety over accidentally mixing interned strings with non-interned strings. This struct prevents that (along with overriding GetHashCode and Equals to be RuntimeHelper.GetHashCode and ReferenceEquals) allowing for much faster Dictionary performance. As for why do I use a struct, I could well use a reference type. But why use a reference type when you're trying to represent a single, immutable, reference value? – Mania Sep 15 '11 at 06:36
  • Behavior explanation: `T x = ...; if (x == null) { .. }` forces `==(T, Nullable)` to be selected as `null` is not a value in `T`, where `T` is a value-type, but is implicitly convertible to `Nullable`. I guess it's slightly better typed than `==(T, Object)`, but it seems sort of shifty-handed and over-clever. ReSharper is using (albeit advanced) heuristics -- and one that misses this sneakiness. –  Sep 15 '11 at 06:39
  • Because with a reference type, a null check would make more sense :) BTW: Immutability has nothing to do with struct vs. reference type. Still, I wouldn't change the semantics of the null check. Instead, you should make sure that you don't allow a StringWrapper with a null string inside and use null StringWrapper instead. You could simplify this using a StringWrapperFactory: `StringWrapperFactory.CreateInternedString(string value){ if(value == null) return null; return new StringWrapper(value); }` – Daniel Hilgarth Sep 15 '11 at 06:40
  • I've ended up doing that, as it also allows me to store the real hash code for the string with the instance (as I expect that to yield better hash performance than the default GetHashCode for reference types), and testing for null then makes a lot more sense as you say. If you'd like to make an answer "just use a class already, and forget this trickery" I'll accept it ;). – Mania Sep 15 '11 at 06:57

1 Answers1

1

I strongly believe in code that is self-documenting, and goes with the principle of least astonishment. For example, "getters" shouldn't modify data, etc. I would consider comparing a struct variable to null fairly confusing at first glance, so I would avoid it (no matter how convenient it might seem). This would be especially true if you were expecting many people to use (or at least look at) your code.

Daniel B
  • 2,877
  • 18
  • 18