The original Ruby code.
1 2 3 4
def filename self.uri.split('/').last end
The original C# code.
1 2 3 4 5 6 7 8 9
public string FileName { get { char[] delim = { '/' }; string[] a = this.Uri.Split(delim); return a[a.Length - 1]; } }
Refactorings
No refactoring yet !
jwmittag
January 1, 2008, January 01, 2008 15:11, permalink
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
This is what the Ruby code would look like in the same style as the C# code.
1 2 3 4 5
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
The original C# code.
1 2 3 4 5 6 7 8 9
public string FileName { get { char[] delim = { '/' }; string[] a = this.Uri.Split(delim); return a[a.Length - 1]; } }
jwmittag
January 1, 2008, January 01, 2008 15:14, permalink
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!
1 2 3 4 5 6 7 8 9
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]; } }
jwmittag
January 1, 2008, January 01, 2008 15:17, permalink
Now we remove the superfluous type annotations. The expressions are so simple that neither the compiler nor we need to see them.
1 2 3 4 5 6 7 8 9
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]; } }
jwmittag
January 1, 2008, January 01, 2008 15:28, permalink
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.
1 2 3 4 5 6 7 8
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]; } }
jwmittag
January 1, 2008, January 01, 2008 15:41, permalink
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.
1 2 3 4 5 6 7 8
public string FileName { get { var a = this.Uri.Split('/'); return a.Last(); // Replace Array indexing with convenience method } }
For reference: The complete MyUri class including the using statement
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25
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 }
jwmittag
January 1, 2008, January 01, 2008 15:57, permalink
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.
The class holding our very own extension method
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25
/// <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]; } } } }
Our MyUri class. Just use our newly defined ExtensionMethods.Array namespace instead of System.Linq
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25
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 }
jwmittag
January 1, 2008, January 01, 2008 16:00, permalink
We can get rid of another temporary local variable and use method chaining instead.
1 2 3 4 5 6 7
public string FileName { get { return this.Uri.Split('/').Last(); // Replace temporary local variable with method chaining } }
jwmittag
January 1, 2008, January 01, 2008 16:18, permalink
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.
1 2 3 4 5 6 7
public string FileName { get { return this.Uri.Segments.Last(); // Replace String splitting with the URI Segments property } }
Our entire refactored class now looks like this
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25
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 }
jwmittag
January 1, 2008, January 01, 2008 16:29, permalink
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?
The original Ruby code.
1 2 3
def filename self.uri.split('/').last end
The final refactored C# code.
1 2 3 4
public string FileName { get { return this.Uri.Segments.Last(); } }
volothamp
July 1, 2008, July 01, 2008 10:13, permalink
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.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20
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(); } } }
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