Good practice to create extension methods that apply to System.Object?

Question

Hello,

I'm wondering whether I should create extension methods that apply on the object level or whether they should be located at a lower point in the class hierarchy. What I mean is something along the lines of:

public static string SafeToString(this Object o) {
    if (o == null || o is System.DBNull)
        return "";
    else {
        if (o is string)
            return (string)o;
        else
            return "";
    }
}

public static int SafeToInt(this Object o) {
    if (o == null || o is System.DBNull)
        return 0;
    else {
        if (o.IsNumeric())
            return Convert.ToInt32(o);
        else
            return 0;
    }
}
//same for double.. etc

I wrote those methods since I have to deal a lot with database data (From the OleDbDataReader) that can be null (shouldn't, though) since the underlying database is unfortunately very liberal with columns that may be null. And to make my life a little easier, I came up with those extension methods.

What I'd like to know is whether this is good style, acceptable style or bad style. I kinda have my worries about it since it kinda "pollutes" the Object-class.

Thank you in advance & Best Regards :)

Christian

P.S. I didn't tag it as "subjective" intentionally.

Answer 1

No, that is not good practice. You want to apply extension methods at the lowest possible point. I believe there a time and a place for (almost) everything, but extension methods System.Object would almost never be appropriate. You should be able to apply extension methods such as this much further down the inheritance stack. Otherwise it will clutter your intellisense and probably end up being used/depended-upon incorrectly by other developers.

However, extension methods for data objects for dealing with Null values is a very good use of extension methods. Consider putting them right on you OleDbDataReader. I have a generic extension method called ValueOrDefault that . . . well, I'll just show it to you:

<Extension()> _
Public Function ValueOrDefault(Of T)(ByVal r As DataRow, ByVal fieldName As String) As T
    If r.IsNull(fieldName) Then
        If GetType(T) Is GetType(String) Then
            Return CType(CType("", Object), T)
        Else
            Return Nothing
        End If
    Else
        Return CType(r.Item(fieldName), T)
    End If
End Function

That's VB, but you get the picture. This sucker saves me a ton of time and really makes for clean code when reading out of a datarow. You are on the right track, but your sense of spell is correct: you have the extension methods too high.

Putting the extension methods into a separate namespace is better than nothing (this is a perfectly valid use of namespaces; Linq uses this), but you shouldn't have to. To make these methods apply to various db objects, apply the extension methods to IDataRecord.

Answer 2

The Framework Design Guidelines advice you not to do this. However, these guidelines are especially for frameworks, so if you find it very useful in your (line of) business application, please do so.

But please be aware that adding these extension methods on object might clutter IntelliSense and might confuse other developers. They might not expect to see those methods. In this case, just use old fashion static methods :-)

---

One thing I personally find troubling about sprinkling extension methods everywhere it that my CPU (my brain) is trained to find possible NullReferenceExceptions. Because an extension method looks like an instance method, my brain often receives a PossibleUseOfNullObject interrupt by the source code parser when reading such code. In that case I have to analyze whether a NullReferenceException can actually occur or not. This makes reading through the code much harder, because I'm interrupted more often.

For this reason I’m very conservative about using extension methods. But this doesn’t mean I don’t think they’re useful. Hell no! I've even written a library for precondition validation that uses extension methods extensively. I even initially defined extension methods on System.Object when I started writing that library. However, because this is a reusable library and is used by VB developers I decided to remove these extension methods on System.Object.

Answer 3

Extension method pollution of System.Object can be quite annoying in the general case, but you can place these extension methods in a separate namespace so that developers must actively opt in to use those methods.

If you couple this with code that follows the Single Responsibility Principle, you should only have to import this namespace in relatively few classes.

Under such circumstances, such extension methods may be acceptable.

Answer 4

An excerpt from the book "Framework design guidelines"

Avoid defining extension methods on System.Object, unless absolutely necessary. When doing so, be aware that VB users will not be able to use thus-defined extension methods and, as such, they will not be able to take advantage of usability/syntax benefits that come with extension methods.

This is because, in VB, declaring a variable as object forces all method invocations on it to be late bound – while bindings to extension methods are compile-time determined (early bound). For example:

public static class SomeExtensions{
     static void Foo(this object o){…} } … Object o = … o.Foo(); 

In this example, the call to Foo will fail in VB. Instead, the VB syntax should simply be: SomeExtensions.Foo(o)
Note that the guideline applies to other languages where the same binding behavior is present, or where extension methods are not supported

Answer 5

Perhaps consider adding the extension methods to the IDataRecord interface? (which your OleDbDataReader implements)

public static class DataRecordExtensions
{
    public static string GetStringSafely(this IDataRecord record, int index)
    {
        if (record.IsDBNull(index))
            return string.Empty;

         return record.GetString(index);            
    }

    public static Guid GetGuidSafely(this IDataRecord record, int index)
    {
        if (record.IsDBNull(index))
            return default(Guid);

        return record.GetGuid(index);
    }

    public static DateTime GetDateTimeSafely(this IDataRecord record, int index)
    {
        if (record.IsDBNull(index))
            return default(DateTime);

        return record.GetDateTime(index);
    }
}

Answer 6

In addition to the reasons already mentioned, it should be borne in mind that extension methods on System.Object are not supported in VB (as long as the variable is statically typed as System.Object and if the variable is not statically typed as Object you should better extend another more specific type).

The reason for this limitation is backward compatibility. See this question for further details.

Answer 7

I know its not best practice, but for my projects I like to include this extension for converting data types, i find it much easier than the Convert class..

public static T ChangeType<T>(this object obj) where T : new()
{
    try
    {
        Type type = typeof(T);
        return (T)Convert.ChangeType(obj, type);
    }
    catch
    {
        return new T();
    }
}

It'll show up in every object but i usually just use this one instead of having a list of extensions..

Works like this...

int i = "32".ChangeType<int>();
bool success = "true".ChangeType<bool>();