Friday, September 21, 2007

Code Voyeur

I like perusing public code repositories, be they CVS/SVN/Mercurial/Git whatever. I'm not sure if this is some kind of sick code voyeurism fetish but I think it's kind of healthy in degrees. It's a good place to pick up tricks. Kind of like doing the weekly ruby quiz but looking at the code before you know what the task is and seeing how closely you think they correlate to the spec.

Scott Hanselman is also on the kick of reading other peoples code to become a better developer, could it be the path to enlightment?

It dawned upon me that at work I have rarely had the need to tread outside my projects. All this time i've been searching far and wide for great code when I had a veritable treasure trove of unread code right under my nose.

We have a VB contractor doing some work for us and I caught wind that someone asked him to do it in C# ASP.NET. Wow. Might not be so bad I thought. So I had a look in SVN and came across this:


 1     public static string Proper(string StringIn) {
 2       string Temp = StringIn;
 3       if (Temp.CompareTo(String.Empty) != 0) {
 4
 5         char[] Delimiter = { ' ' };
 6         string[] Words = Temp.Split(Delimiter);
 7         Temp = String.Empty;
 8
 9         for (int i = Words.GetLowerBound(0); i <= Words.GetUpperBound(0); i++)
10           if (Words[i].Trim().CompareTo(String.Empty) != 0)
11             Temp += Words[i].Substring(0, 1).ToUpper() +
12                     Words[i].Substring(1).ToLower() + " ";
13         return Temp;
14       }
15       else
16         return String.Empty;
17     }
18


Apart from the upper case local variable names, in-efficient string concatenation and re-inventing the wheel, the dude did alright. Absolute respect to him, he's smart, he got it working, but it is obvious that C# is not his native tongue. That is textbook VB in C#.

Here is something roughly equivalent I came up with that produces the same output (don't lambast me for not getting the current culture, "en-AU" does weird shit that "en" doesn't) :)


1     using System.Globalization;
2     public static string TitleCase(string str) {
3       return String.IsNullOrEmpty(str) ? String.Empty
4              : new CultureInfo("en").TextInfo.ToTitleCase(str.ToLower());
5     }


It's not his fault really. He was asked to code in a language he wasn't familiar with and he got by. But what he did was speak English in down-town Nairobi.

Now I don't know what my point is but just look at the API's if you are using an unfamiliar language. I'm not shit-hot with C#, give me C, Java or Ruby and I've got a fighting chance. The thing is I'd assume that .NET would provide a way to title case a string. Instead of re-inventing the wheel I did a google search and came across the TextInfo class. You think he would have twigged that if VB has StrConv there might be something vaguely similar in C#.

Moral of the story, if you're asked to code in a language you are not familiar with, knock it back or make it clear it might not look pretty. But i guess who doesn't like getting paid?

2 comments:

Unknown said...

I think when you work with frameworks as big as .NET (or Java or Ruby or whatever) and you're tasked to do something which isn't business specific - you should first assume someone already did it and google it or search in the docs before you continue.

Arnon

dan said...

Yeah, I agree that's a smart way to go about it arnon, good tip.