89f45d140c5d9b14fd63720ea8b0cb47

On his Blog "Progress vs. Perfection" <http://Progress-vs-Perfection.BlogSpot.Com/>, Scott Porad posted a small comparison between a simple method for getting the Filename segment of a URI, implemented in Ruby and in C#. Read first: "Rails vs. ASP.NET (Really, Ruby vs C#)" <http://Progress-vs-Perfection.BlogSpot.Com/2007/12/rails-vs-aspnet-really-ruby-vs-c.html>.

While I agree with his basic premise that C# is much more verbose than Ruby, I think that in this particular example the comparison is a bit unfair, because the C# version

* uses unnecessarily complex types,
* uses superfluous type annotations,
* passes parameters in temporary local variables while the Ruby version passes them directly as a literal,
* uses temporary local variables while the Ruby version uses method chaining,
* uses Array indexing while the Ruby version uses convenience methods and, last but not least,
* uses the wrong type for the URI (a URI is *not* a String!).

Basically, the Ruby and the C# version are written in totally different styles and are thus not really comparable.

Anyway, I have long wanted to play around with C# and this seemed like the perfect excuse, so I downloaded Visual Studio and got going! Please keep in mind that until this very minute I have never ever written a single line of C# or .NET before, so be gentle. But, hey, improving code is what this site is all about, right?

I'm going to post the original code here and then reply with a series of simple refactorings, hoping that others will chime in with their own!

jwm

def filename
  self.uri.split('/').last
end
public string FileName
{
    get
    {
        char[] delim = { '/' };
        string[] a = this.Uri.Split(delim);
        return a[a.Length - 1];
    }
}

Refactorings

No refactoring yet !

89f45d140c5d9b14fd63720ea8b0cb47

jwmittag

January 1, 2008, January 01, 2008 15:11, permalink

1 rating. Login to rate!

To make the Ruby and C# versions more comparable, they would both have to be written in the same style. This is what the Ruby code would look like if it was written in the same style as the C# code.

It is however better to improve the C# code than to uglify the Ruby code, don't you think? (-;

jwm

def filename
  delim = '/'               # temporary local variable instead of passing as a literal parameter
  a = self.uri.split(delim) # temporary local variable instead of message chaining
  a[-1]                     # Array indexing instead of convenience methods
end
public string FileName
{
    get
    {
        char[] delim = { '/' };
        string[] a = this.Uri.Split(delim);
        return a[a.Length - 1];
    }
}
89f45d140c5d9b14fd63720ea8b0cb47

jwmittag

January 1, 2008, January 01, 2008 15:14, permalink

1 rating. Login to rate!

Let's now work on improving the C# code.

First step: remove unnecessarily complex types. We really don't need an Array to hold a single character!

public string FileName
{
    get
    {
        char delim = '/';                   // Use a char instead of a char array
        string[] a = this.Uri.Split(delim);
        return a[a.Length - 1];
    }
}
89f45d140c5d9b14fd63720ea8b0cb47

jwmittag

January 1, 2008, January 01, 2008 15:17, permalink

No rating. Login to rate!

Now we remove the superfluous type annotations. The expressions are so simple that neither the compiler nor we need to see them.

public string FileName
{
    get
    {
        var delim = '/';               // Use local variable type inference to get rid of type annotations
        var a = this.Uri.Split(delim); // Use local variable type inference to get rid of type annotations
        return a[a.Length - 1];
    }
}
89f45d140c5d9b14fd63720ea8b0cb47

jwmittag

January 1, 2008, January 01, 2008 15:28, permalink

No rating. Login to rate!

Introducing temporary local variables can often increase readability but in this case it's just unneeded complexity: it's fairly obvious that Split() takes a delimiter to split on and it's also obvious that '/' is the path delimiter in URIs. Get rid of it.

public string FileName
{
    get
    {
        var a = this.Uri.Split('/'); // Directly pass the delimiter as a char literal instead of using a temporary local variable
        return a[a.Length - 1];
    }
}
89f45d140c5d9b14fd63720ea8b0cb47

jwmittag

January 1, 2008, January 01, 2008 15:41, permalink

No rating. Login to rate!

The Ruby version uses the Array#last convenience method instead of Array indexing to make the code more expressive. We can do the same in C#. Unfortunately the Last() convenience method is not available in the default namespace, but we can import it from the System.Linq namespace after adding a reference to the System.Core assembly.

public string FileName
{
    get
    {
        var a = this.Uri.Split('/');
        return a.Last();             // Replace Array indexing with convenience method
    }
}
using System.Linq;
class MyUri
{
    #region Boring boilerplate stuff
    public MyUri(string uri) { this.Uri = uri; }

    private string uri = "http://LocalHost/";
    public string Uri { get { return uri; } set { uri = value; }
    }
    #endregion Boring boilerplate stuff

    #region The meat of the example
    /// <summary>
    /// Get the Filename segment of the URI.
    /// </summary>
    public string FileName
    {
        get
        {
        var a = this.Uri.Split('/');
        return a.Last();
        }
    }
    #endregion The meat of the example
}
89f45d140c5d9b14fd63720ea8b0cb47

jwmittag

January 1, 2008, January 01, 2008 15:57, permalink

No rating. Login to rate!

Importing the entire System.Linq namespace might be overkill, just to get the Last() convenience method. Let's extend the Array type ourselves and write our own Last() method.

/// <remarks>
/// This namespace holds our own extension methods.
/// </remarks>
namespace ExtensionMethods
{
    /// <remarks>
    /// This namespace holds our extension methods for the Array class.
    /// </remarks>
    namespace Array
    {
        /// <remarks>
        /// This class holds extension methods for the Array class.
        /// </remarks>
        static class ArrayExtensions
        {
            /// <summary>
            /// Get the last element of an Array.
            /// </summary>
            public static TSource Last<TSource>(this TSource[] source)
            {
                return source[source.Length - 1];
            }
        }
    }
}
using ExtensionMethods.Array; // Use our newly written extension method
class MyUri
{
    #region Boring boilerplate stuff
    public MyUri(string uri) { this.Uri = uri; }

    private string uri = "http://LocalHost/";
    public string Uri { get { return uri; } set { uri = value; }
    }
    #endregion Boring boilerplate stuff

    #region The meat of the example
    /// <summary>
    /// Get the Filename segment of the URI.
    /// </summary>
    public string FileName
    {
        get
        {
            var a = this.Uri.Split('/'); // Directly pass the delimiter as a char literal instead of using a temporary local variable
            return a.Last();
        }
    }
    #endregion The meat of the example
}
89f45d140c5d9b14fd63720ea8b0cb47

jwmittag

January 1, 2008, January 01, 2008 16:00, permalink

No rating. Login to rate!

We can get rid of another temporary local variable and use method chaining instead.

public string FileName
{
    get
    {
        return this.Uri.Split('/').Last(); // Replace temporary local variable with method chaining
    }
}
89f45d140c5d9b14fd63720ea8b0cb47

jwmittag

January 1, 2008, January 01, 2008 16:18, permalink

No rating. Login to rate!

Last but not least we can use a proper URI type to represent our URI instead of a String. After all, URIs *aren't* Strings, they are URIs!

By making our uri member a Uri type instead of a String, we get rid of the String splitting and can use the Segments property instead, which splits up the Uri for us.

public string FileName
{
    get
    {
        return this.Uri.Segments.Last(); // Replace String splitting with the URI Segments property
    }
}
using System;                                                 // The System namespace contains the Uri type
using ExtensionMethods.Array;
class MyUri
{
    #region Boring boilerplate stuff
    public MyUri(string uri) { this.Uri = new Uri(uri); }     // The constructor now builds a Uri from its String argument instead of directly setting the property

    private Uri uri = new Uri("http://LocalHost/");           // The uri variable now has type Uri
    public Uri Uri { get { return uri; } set { uri = value; } // The Uri property now has type Uri
    }
    #endregion Boring boilerplate stuff

    #region The meat of the example
    /// <summary>
    /// Get the Filename segment of the URI.
    /// </summary>
    public string FileName
    {
        get
        {
            return this.Uri.Segments.Last();                  // Replace String splitting with the URI Segments property
        }
    }
    #endregion The meat of the example
}
89f45d140c5d9b14fd63720ea8b0cb47

jwmittag

January 1, 2008, January 01, 2008 16:29, permalink

2 ratings. Login to rate!

If we now reformat the C# code a bit, it compares quite well to the Ruby code. The Ruby version still wins hands down, because the necessary boilerplate code to create the property makes the C# snippet a little noisy, but the difference is not nearly as extreme as before.

What do you think?

def filename
  self.uri.split('/').last
end
public string FileName
{
    get { return this.Uri.Segments.Last(); }
}
Cd40128e044f39d7063b5cfdeace80f6

volothamp

July 1, 2008, July 01, 2008 10:13, permalink

No rating. Login to rate!

This is very succint.

I will do it in this way, since I think it's better to have this filename method for all the strings (to avoid refactoring if we used string in all the program)

You should manage exception. The Uri() constructor throws it in the case that the input string is not a valid URI.

Ruby version hasn't got the exception management, so I think C# version is more solid too.

Bye.

using System;
using System.Linq;

namespace UriRefactor
{
    static class StringExtension
    {
        public static string FileName(this string s) {
            return new Uri(s).Segments.Last();
        }
    }

    class Program
    {
        static void Main(string[] args)
        {
            var f = "http://localhost/".FileName();
        }
    }
}
1050ca7d4dbf1ceb9c480820039344f0

Ben Sapp

January 10, 2010, January 10, 2010 08:50, permalink

No rating. Login to rate!

Are you sure the last segment will always be a filename?

Your refactoring





Format Copy from initial code

or Cancel