Assumptions and Legacy Code

Yesterday I encountered an interesting snippet when going over some legacy code that really made me think about my assumptions.

if (ipAddress.Length > 15)
{
    ipAddress.Substring(0, 14);
}

This should be pretty clear to most folks, if the IP address string is longer than 15 characters, truncate it to 15. Theoretically, the longest an IPv4 address could be is 15 characters (000.000.000.000) if written in base 10.

Determining Intent

Why would we ever need to truncate it to 15 if this is the case?

One of my colleagues raised a good possibility: what if that IP address contained additional information, such as a port number? It could then appear as 000.000.000.000:00000. That makes some sense, but what about the many cases where each octet (group of 000) is less than three digits? In that case some of a port number may be included in the resultant string and it wouldn’t make a whole lot of sense to do that.

Besides, if you just wanted to drop the port number, why wouldn’t you use regex to trim the string properly? Not that I love regex, but it does have its uses.

“I had a problem, so I used regex. Now I have two problems.”

Another idea surfaced from a different colleague (don’t odd pieces of code attract developers like flies to honey?): maybe the actual value of the string is unimportant and the database is limited to 15 characters? That also wasn’t the answer.

It seems that if you wanted to get an IP address from a string in .Net your best choice would be to use IPAddress.Parse() which will parse IPv4 and IPv6. If that didn’t work because you had port information, you could parse it as a URI and extract the IP address that way.

It’s also worth noting that this snippet of code contained no comments and has no unit tests. There’s no real way to extract the intent from the code. This is a perfect example of a spot where a comment could indicate intent to future developers for maintenance. It’s also a perfect example of how well-defined unit tests can help provide intent in the same way, while also showing whether or not it is safe to alter the code.

Assumptions

You might have noticed that this post has “Assumptions” in the title, but so far I haven’t discussed any.

There are two possible assumptions that one could make here:

  1. Clearly the person who wrote this code had no idea what they were doing.
  2. The person who wrote this code is smarter than I am and wouldn’t have written this without a good reason.

(Ok, there are really a lot of other possibilities for assumptions, but these are the two I flip between.)

In the case where you are working on a defect, and the code in question seems to be the problem, the first assumption may be a safe bet. Really though, even then you must assume that you can’t just rip the code out and instead figure out why the code doesn’t work as intended, meaning you must still determine the intent.

In the case where I am just cleaning up code that works, the second assumption is always my choice. I am not aware of any defects caused by this section of code, which is not to say that they don’t exist. So it is far safer to assume that someone put this code in to resolve a defect. They likely believed that it was obvious what they were doing, and didn’t comment the code. They probably believed that the code was simple enough that a test was unnecessary and it didn’t need to be separated into a well-named method.

I don’t agree with the assumptions I’m led to believe they made, but I do find myself thinking that they must have been smart enough to know what they were doing and that they had a completely valid reason.

In the end, the code works. I’m not going to alter it. I’m not going to rip it out. All that’s going to do is risk reintroducing a bug that was fixed a long time ago. I’d like to add unit tests to cover this, but without know the requirements, a proper unit test would be hard to write. I could extract the code into a separate method, but I’d have to make assumptions about what the intention of the code was so that it could be named properly. Naming the extracted method something similar to TrimIpAddress is going to make future developers believe they know the intent of the code and can test and change it freely.

In the end, the safest thing to do is to leave it in peace and let it keep doing whatever it is it’s doing.

The Lessons I’ve Learned from Rocky

I’ve watched the Rocky films a lot, a lot. I’ve always just found them to be a lot of fun, but at the same time the story of the underdog has always been extremely compelling for me. I hate being told what I can and can’t accomplish and that’s usually what compels me to achieve. There’s five great Rocky films, so lots of lessons to take away. Here I’ll provide one from each, because that’s just how I roll. (I’m not including Rocky V because it’s the only Rocky film I don’t care for.)

Rocky

“Cause I was thinkin’, it really don’t matter if I lose this fight. It really don’t matter if this guy opens my head, either. ‘Cause all I wanna do is go the distance. Nobody’s ever gone the distance with Creed, and if I can go that distance, you see, and that bell rings and I’m still standin’, I’m gonna know for the first time in my life, see, that I weren’t just another bum from the neighborhood.”

Rocky was all about going the distance. Winning didn’t even matter to Rocky because he knew he was going to lose the fight.

The primary take away for me from Rocky is that if you don’t complete the things that you start, you don’t even have a chance. How can you ever be anything important if you don’t finish?

I really think this is the most important lesson of all the films and it’s something that I strive for constantly because I have a bad habit of starting personal projects and then never releasing them. If I were to continue on that road, how could any of them ever be successful?

Rocky II

Apollo: [Apollo is reading fan mail] Mary Anne, you listen to this. “You didn’t beat nobody and anybody who knows boxing knows the fight was fixed.” This one came from London. “You call yourself the champ? You’re a fake! The fight was a fake. Go kill yourself!”

Mary Anne Creed: Wouldn’t you rather play with the children than read hate mail?

Apollo: “How much did you get to carry that bum for 15 rounds? You are a disgrace to your people.”

Mary Anne Creed: Why can’t you ignore it?

Apollo: Are you serious?

Apollo really gets a rough time in this movie, and he doesn’t deserve it. In the first movie, he’s a nice guy. He’s giving a nobody a shot at the title — even though he really just thinks it’s a good publicity gimmick. His biggest failing is that he can’t let go of the criticism he’s receiving. This even leads to him losing the world champion title.

It’s important to pay attention to what your critics are saying. You need to analyze it and see if there’s anything there for you to consider. Constructive criticism is the best source of data for self improvement. What you don’t need is to take hateful criticism to heart. That will only serve to bring you down.

Rocky III

“Well, Rock, let’s put it this way. Now, three years ago you was supernatural. You was hard and you was nasty and you had this cast-iron jaw but then the worst thing happened to you, that could happen to any fighter. You got civilized. But don’t worry kid. You know, presidents retire, horses retire, Man-o-war retired. They put him out to stud. That’s what you should’ve done, retired.”

The lesson here isn’t really related to the quote, although the quote raises a good point as well. If you don’t have the desire to make something happen, then it’s not going to happen.

Instead, my take away here is that you shouldn’t attempt to do something critical when you’re in a poor state of mind. In this movie, Rocky’s manager — a huge father figure to him — gets a heart attack right when Rocky is about to go into a title defense fight. Rocky spends some time with him and knows things aren’t well, but goes to fight anyway. His heart and his mind aren’t in that fight, they’re with his manager. As expected, he loses. This can be applied to everyone — why are you trying to finish that critical piece of your work when you are tired, sick, have critical life issues, or are otherwise extremely distracted? Your distraction will only cause problems with the work that you are doing. Don’t even attempt it.

For me, the scariest part of working when I’m distracted is when the code compiles perfectly the first time. It only means that the defects are more subtle and are going to be harder to find.

Rocky IV

“I guess what I’m trying to say is, if I can change, and you can change, everybody can change.”

Never get too tied into what you’re doing. Always remain open for change. The only people who aren’t willing to change are the people who aren’t going anywhere (except sometimes they’re being forced out). Enough said.

Rocky Balboa

“Let me tell you something you already know. The world ain’t all sunshine and rainbows. It’s a very mean and nasty place and I don’t care how tough you are it will beat you to your knees and keep you there permanently if you let it. You, me, or nobody is gonna hit as hard as life. But it ain’t about how hard ya hit. It’s about how hard you can get hit and keep moving forward. How much you can take and keep moving forward. That’s how winning is done!”

Rocky Balboa was really a movie about proving that no matter how old you are, you can still accomplish your goals. This isn’t really a valuable lesson for me to apply to myself since I’m still reasonably young. It does remind me that I should not jump to ageism when considering the abilities of others. Younger people fail just as hard as older people. Competent people come from all generations since competence is not tied to age.

The quote here was a great quote, but it really applies to all of the movies. Life is not always pretty. Maybe you’ve read Oh, The Places You’ll Go and maybe not, but it covers the same concept. If you don’t have the mind to put in the efforts during the hard parts of life then you won’t be able to enjoy the highs when they come by. You will forever be focused on the lows and that’s no way to live. Keep pressing forward and the skies will eventually clear.

Goals for 2014

* Blog a little more (not going well so far)

I wanted to blog a lot in 2013. I didn’t get there. I produced an average of one post every two weeks when averaged out across the year. In reality that looks like a lot less than one post every two weeks because there are a lot of gaps.

I did have a couple blog posts this year that have been reasonably popular in search engines, and I’d like to continue on that track. I’d also like to move the blog to its own domain.

My goal for 2014 is to produce two blog posts per month at a minimum. This keeps me on track for the same amount of content produced as in 2013, but on a more regular schedule. This is in part to increase my personal brand, and also to increase the amount of writing I’m doing and improve the quality of that writing.

* Read a book a month

In 2013, I read some fiction books, but that’s not really what I’m talking about here. I did finish Code Complete 2 by Steve McConnell and that was a wonderful book. I also started a lot of non-fiction professional development books that I didn’t finish. In fact, I’m collecting a nice little library of these books.

These year I want to finish some of them. First on my list is The Pragmatic Programmer by Andrew Hunt and David Thomas. I’m about halfway through it currently and it’s an excellent book. After that I plan on finishing Peopleware Third Edition by Tom DeMarco and Timothy Lister, what I’ve read of this book so far is excellent.

I’ve got about six books lined up after these ones, but I’m making sure to focus my attention in order to actually get these finished.

I’m also halfway through The Count of Monte Cristo, but again, not really talking about fiction books here.

* Finish something

I finish a lot of stuff at the office. This is referring to personal projects. In 2013, I’ve finished two personal projects. One was not developed as completely as I want it to be. The second is publicly available but has never really been “launched.” The main reason for this is that the design isn’t where I want it.

So this year I would like to launch another personal project. One of the key things I’m going to be developing in 2014 are my front end design skills. This will allow me to be happier — if never completely happy — with my sites and apps so that I don’t feel so bad about actually launching them.

* Give some more talks

I don’t know how much I really learned in high school.

I didn’t take away good study habits. I didn’t learn how to be a good person. I didn’t learn how to socialize. All of the computer knowledge I gained during that time was gained myself, not through classes.

But I know that I learned one thing that will stick with me forever – public speaking. I had a teacher for grade 11 English, Mr. Doddington. Now, he was an ecentric man, but he was also passionate about speaking. He had everyone in the class constantly giving speeches to the room. During these speeches everyone in the class was encouraged to keep track of how many “um”s, “uh”s, and “like”s you uttered. Everyone was forced to continue until they no longer stuttered these words during their speeches.

Today I can’t help myself. Everytime someone gives a talk I’m wired to listen for these moments, almost everyone has them and it drives me crazy. Mr. Doddington gave me a pretty awesome gift in high school — the ability to talk without stuttering. I’ll forever be grateful.

I gave five presentations to the engineering department at the office in 2013. In 2014, I’d like to make further use of this gift by continuing to present internally and also by giving one talk externally.

* More responsibility at work

This one is self-explanatory. I’m really happy working where I am now. I’m happy with the work. I’m happy with my coworkers. I’m even happy with my manager.

I’d like to continue on my career path and take on more responsibility this year. I’ve never been very happy staying stagnant somewhere, and if I remain stagnant where I am then my personal history has shown that I’ll be getting the itch for something new by 2015.

* Passive income

This is another self-explanatory item.

I’ve got a family at home and I’d like to bring in more revenue in 2014. I’m also not keen on working 100 hours a week. So it’s time to build some stuff that brings in passive income to bolster my finances. This also ties in with “Finish something”.

Well, there are my goals for 2014. At the end of the year I’ll do a report card and see how I measure up. Regardless, I think it’s going to be a pretty great year.

Why Design by Committee Doesn’t Work for Me

Platypus

To me, the platypus looks like an animal that was designed by a committee. Here’s how I picture their design meeting went.

Project Leader: Hey everyone, we’re going to make a new animal. It’s going to be amazing! It’s going to be everything we love. So, let’s go around and here what everyone wants.

Joe: I love dogs. I think it should be like a small, furry dog.

Marsha: I love beavers. It should definitely have a flat tail like a beaver.

Jim: I love ducks. Can it have a duck bill and webbed feet?

Project Leader: It can have whatever you want, Jim.

Louise: I just had a baby and giving birth is kinda messy. Let’s make sure it lays eggs.

Stan: I love snakes. This thing needs some poisonous venom to defend itself.

Chris:

Marsha: You know, Chris got hit on the head really hard this morning, maybe he shouldn’t be involved.

Project Leader: Nonsense. Everyone gets a say. Well, Chris?

Chris: … I want it to… sweat milk.

Project Leader: You got it! Ok. Thanks for your input, everyone. I’ll get this done.

Don’t get me wrong, I think the platypus is a pretty neat creature. It’s definitely odd though. Odd enough that it seems to like a unifying concept.

If there was a design meeting to come up with dogs I imagine it would go quite differently. The project leader would come in with something already designed and ask instead for feedback on his current design. When different people have different requests, he accommodates simply by coming up with different breeds of dogs instead of trying to fit everyone’s vision into one animal.

This isn’t all hypothetical

I’ve been part of meetings where the product started out without a design. The developer simply brought all of the potential end users into a room and asked them how they wanted the product to look and behave. Everyone had a say. What came out of that meeting was a pretty terrible mess.

Ok, I lied when I said that everyone had a say. I didn’t have a say. I was a senior member of the team that would be consuming this product and I was heavily involved in training new additions to the team. I didn’t say anything because I was able to identify pretty early that this design was going to be a mess.

So what did I do about it? After the meeting, I sat down with the development team — just me and them. I talked about the problems that we were facing, how the product should logically be laid out to increase productivity and decrease context switching, and why I had made these decisions. Needless to say, the product ended up being built to my specification.

Once a prototype was completed, we then went back to the rest of the team and walked them through it. Asking for input only once the walkthrough was done and the decisions had been explained. There was very little dissent amongst the group and for the most part the design went unchanged and people were excited about what the product was going to be.

I don’t think that I was the main factor in the success of this design. I believe it was simply that there was an overall vision, and one that could be explained logically before seeking input. Many people have a problem working with a blank canvas. There is no base there for people to build on or tweak. Everyone ends up seeing something wildly different, and bringing that together successfully is near impossible without disappointing something.

Not to mention that if something designed by committee has regular checkpoints then it is going to have more requested modifications. In the end, that product is going to cost more and is going to be less accepted.

Creating a Generic Type Converter for C#

Recently I was given the task of creating a type converter that would automatically convert objects between abstraction layers without using dedicated DTOs (data transfer objects). I was also given the limitation of not using dedicated constructors that accept another type and do the conversion manually.

The approach that was suggested to me used custom attributes to define which properties mapped to other properties. I’m not really a fan of custom attributes though — don’t get me wrong, they certainly have their place, but I find that they can make reading code more difficult if you don’t know what their functionality and how they are being manipulated through reflection.

My solution was thus:

  • accept an in object and an out object
  • iterate over all publicly exposed properties of the in object
    • generate a Dictionary of their name to their value
  • iterate over all publicly exposed properties of the out object
    • for any properties named the same, copy the value over

Here’s the code that accomplished this:

    public class TypeConverter
    {
        public TypeConverter()
        { }

        public T2 ConvertTypes<T1, T2>(T1 convertFrom, T2 convertTo)
        {
            var convertFromType = convertFrom.GetType();
            var propertyValues = new Dictionary<string, object>();
            var neededProperties = convertFromType.GetProperties();
            foreach (var property in neededProperties)
                propertyValues.Add(property.Name, property.GetValue(convertFrom, null));

            var convertToType = convertTo.GetType();
            var outputProperties = convertToType.GetProperties();
            foreach (var property in outputProperties)
                if (propertyValues.ContainsKey(property.Name))
                    property.SetValue(convertTo, propertyValues[property.Name], null);

            return convertTo;
        }
    }

This worked as advertised, but another question came up — what about conversions that are not 1:1? What if a property called Revision was a string in the object being converted from and an int in the object being converted to? Well, back to the drawing board.

This time I wanted to make sure that I am able to register non-standard conversions with the converter. When iterating over the properties of the outbound object, the property name and conversion types need to be checked to see if any custom conversions have been registered and use that conversion if one exists.

Here’s what I came up with:

    public class TypeConverter
    {
        public delegate dynamic ConvertProperty 
            (dynamic convertFrom, dynamic convertTo, string propertyName);
        private Dictionary<int, ConvertProperty> _customizations;

        public TypeConverter()
        {
            _customizations = new Dictionary<int, ConvertProperty>();
        }

        public void Register(Type convertFrom, Type convertTo, string propertyName, ConvertProperty convert)
        {
            int hash = new ConvertKey(convertFrom, convertTo, propertyName).GetHashCode();
            if (!_customizations.ContainsKey(hash))
                _customizations.Add(hash, convert);
        }

        public T2 ConvertTypes<T1, T2>(T1 convertFrom, T2 convertTo)
        {
            var convertFromType = convertFrom.GetType();
            var propertyValues = new Dictionary<string, object>();
            var neededProperties = convertFromType.GetProperties();
            foreach (var property in neededProperties)
                propertyValues.Add(property.Name, property.GetValue(convertFrom, null));

            var convertToType = convertTo.GetType();
            var outputProperties = convertToType.GetProperties();
            foreach (var property in outputProperties)
            {
                if (propertyValues.ContainsKey(property.Name))
                {
                    int hash = new ConvertKey(convertFromType, convertToType, property.Name)
                        .GetHashCode();
                    if (_customizations.ContainsKey(hash))
                        property.SetValue(convertTo, _customizations[hash](convertFrom, convertTo, property.Name), null);
                    else
                        property.SetValue(convertTo, propertyValues[property.Name], null);
                }
            }

            return convertTo;
        }

        private class ConvertKey
        {
            private Type _convertFrom;
            private Type _convertTo;
            private string _propertyName;

            public ConvertKey(Type convertFrom, Type convertTo, string propertyName)
            {
                _convertFrom = convertFrom;
                _convertTo = convertTo;
                _propertyName = propertyName;
            }

            public override int GetHashCode()
            {
                unchecked
                {
                    return _convertFrom.GetHashCode() * _convertTo.GetHashCode() 
                        * _propertyName.GetHashCode();
                }
            }
        }
    }

As you can see, there is now a Dictionary internal to the class which holds an int key and a ConvertProperty delegate — which takes a from type, to type, and property name.

A Register method has been added to register new custom conversions. It calculates a hash to use as the Dictionary key, and if that key does not already exist, adds the delegate to complete the key value pair.

That hash key is a simple unchecked calculation of the hash codes of the two types multiplied with the hash code of the property name.

The last addition is that, as mentioned, when iterating over outbound properties a hash is calculated and checked against the dictionary and if needed, the custom conversion is run.

Here is a test in action:

        public void StraightConvert()
        {
            var convertFrom = new SampleClass1() { Num = 1 };
            var result = _converter.ConvertTypes(convertFrom, new SampleClass2());
            Assert.AreEqual(result.Num, convertFrom.Num);
        }
        public void ConvertWithRegister()
        {
            _converter.Register(typeof(SampleClass1), typeof(SampleClass2), "Num", (cF, cT, pN) =>
            {
                var cFType = cF.GetType();
                return (int)cFType.GetProperty(pN).GetValue(cF, null) * 10; 
            });

            var convertFrom = new SampleClass1() { Num = 1 };
            var result = _converter.ConvertTypes(convertFrom, new SampleClass2());
            Assert.AreEqual(convertFrom.Num * 10, result.Num);
        }

But the capabilities of this are greater than just multiplying by 10. It would be very easy to have the input type be a list of dates, from which the max date is pulled, as an example. Any type of executable code you need can be registered with the converter.

By far, this is one of the most fun pieces of code I’ve written in C# to date.

OpenGIS – Encoding KML Files in C#

KML is an OpenGIS encoding standard that allows for encoding points and shapes on a map. It’s similar to a shapefile, but a KML file’s points are based on latitude and longitude, and a shapefile’s points are based on a flattened offset coordinate system. For a lot of purposes this means KML is easier to read and generate.

Earth

Fortunately KML files also import into a lot of GIS applications and are supported by Google Earth.

KML is stored in XML, so to encode it we can use .Net’s XML serialization libraries.

For this example we are just going to be encoding an array of points, or placemarks. These consist of a name, description, and location. The locations consist of latitude, longitude, and altitude.

Here are the data models for placemarks and locations:

public class Placemark
{
    public Placemark(string rawData)
    {
        string[] tokens = rawData.Split(',');
        Name = tokens[1];
        Description = "Place ID: " + tokens[0];
        Point = new Location()
        {
            Longitude = Double.Parse(tokens[2],
                CultureInfo.InvariantCulture),
            Latitude = Double.Parse(tokens[3],
                CultureInfo.InvariantCulture),
        };
    }

    public Placemark()
    { }

    [XmlElement("name")]
    public string Name { get; set; }

    [XmlElement("description")]
    public string Description { get; set; }

    [XmlElement("Point")]
    public Location Point { get; set; }
}
public class Location
{
    private const string TOKEN = ",";

    public Location()
    {
        Altitude = 0;
    }

    [XmlIgnore()]
    public double Latitude { get; set; }
        
    [XmlIgnore()]
    public double Longitude { get; set; }

    [XmlIgnore()]
    public int Altitude { get; set; }

    [XmlElement("coordinates")]
    public string Coordinates 
    {
        get
        {
            var sCoordinates = new StringBuilder();
            sCoordinates.AppendWithToken(Longitude.ToString(), TOKEN); 
            sCoordinates.AppendWithToken(Latitude.ToString(), TOKEN);
            sCoordinates.Append(Altitude.ToString());
            return sCoordinates.ToString();
        }
        set { }
    }
}

So in the Placemark class you can see that I’m providing it a comma-separated string in the constructor. This is simply because my input data is going to be a csv row. This does not follow good dependency design, but it does make it easier to work within the confines of the small program I’ve constructed. If I ever needed to build this out, I would change this.

The first token in the input string is the description, then comes the name, longitude, and latitude. We are not specifying the altitude although that could be done easily. I simply don’t have that data. Instead I force the altitude to 0 in the Location constructor.

In the Location class the only property that needs to be serialized is the Coordinates string. This string is a comma separated string containing the longitude, latitude, and altitude in that specific order. There is a setter there, but it does not actually do anything. It’s just there to make the serializer happy.

I’m using a PlacemarkCollection class as the root of the XML being serialized. The code for that is simple.

[XmlRoot("kml")]
public class PlacemarkCollection
{
    public PlacemarkCollection()
    {
        Document = new List<Placemark>();
    }

    [XmlArrayItem("Placemark")]
    public List<Placemark> Document { get; set; }

    public void Add(Placemark toAdd)
    {
        Document.Add(toAdd);
    }
}

I’m using a basic class to transform a CSV file into a PlacemarkCollection.

public static class CSVTransformer
{
    public static string[] ReadCsvFile(string fileName)
    {
        string[] source = File.ReadAllLines(fileName);
        return source;
    }

    public static PlacemarkCollection CreatePlacemarkCollection(string[] input)
    {
        var collection = new PlacemarkCollection();
        foreach (string rawData in input)
        {
            collection.Add(new Placemark(rawData));
        }

        return collection;
    }
}

There is an Add right on the collection class to work better with the Liskov principal. This way the CSVTransformer does not need to care about how the PlacemarkCollection is implemented.

Then we have the main program which provides the workflow for the program.

public class Program
{
    private const int INPUT_FILE = 0;
    private const int OUTPUT_FILE = 1;
    private const string DEFAULT_INPUT_FILE = "input.csv";
    private const string DEFAULT_OUTPUT_FILE = "output.kml";

    private PlacemarkCollection ImportAndProcessData(string fileName)
    {
        string[] input = CSVTransformer.ReadCsvFile(fileName);
        return CSVTransformer.CreatePlacemarkCollection(input);
    }

    public Program()
    { }

    /// <summary>
    /// The first arg is input file name. 
    /// The second arg is output file name.
    /// </summary>
    static void Main(string[] args)
    {
        string inputFileName = args.Count() < 1 ? 
            DEFAULT_INPUT_FILE : args[INPUT_FILE];
        string outputFileName = args.Count() < 2 ? 
            DEFAULT_OUTPUT_FILE : args[OUTPUT_FILE];
            
        var kmlExporter = new Program();
        var data = kmlExporter.ImportAndProcessData(inputFileName);
        var serializer = new PlacemarkCollectionSerializer();
        serializer.Serialize(outputFileName, data);
    }
}

So the program allows the user to specify an input and output file if run from the command prompt. And if it is not specified then defaults are provided. It is not currently possible to have a specific output and a default input, but that optimization would be easy enough to add.

So the data is processed by the CsvTransformer, and then serialized to the output file. Super easy. The only thing we haven’t looked at is the serializer.

public class PlacemarkCollectionSerializer
{
    private const string KML_NAME_SPACE = "http://www.opengis.net/kml/2.2";

    public void Serialize(string fileName, PlacemarkCollection placemarks)
    {
        var serializer = new XmlSerializer(typeof(PlacemarkCollection),
            KML_NAME_SPACE);
        using (var stream = new FileStream(fileName, FileMode.Create))
        {
            using (var writer = new XmlTextWriter(stream, Encoding.Unicode))
            {
                var namespaces = new XmlSerializerNamespaces();
                namespaces.Add(string.Empty, KML_NAME_SPACE);
                serializer.Serialize(writer, placemarks, namespaces);
            }
        }
    }
}

It is critically important that the KML name space be added during serialization. This is at http://www.opengis.net/kml/2.2. Other than that we just use a FileStream, XmlTextWriter, and the .Net XmlSerializer to serialize the output.

There’s really nothing difficult or tricky about this whole process. KML is a well defined format and .Net’s libraries make it reasonably trivial to implement.

For more information on KML the reference is here: https://developers.google.com/kml/documentation/kmlreference
And Google provides documentation and a developer guide here:

If you currently do not offer any location services, often customers are happy to receive KML files and it is a fairly easy feature to implement.

C# – Creating a Shell Extension for Review Board

Recently at work we’ve started using Review Board to request and complete code reviews. This has been a nice way to replace our formerly informal code review process of calling someone over to your desk to review.

Along with Review Board we’ve decided to use a tool called “post-review”. This is a command line tool to easily publish a review to Review Board.

The only problem with this is that as much as I like command line tools, I’m never in the command line. I don’t have a memory for switches, and because the command line is never open I’ve got to go through the process of opening the command line first. All of this seemed like just a little too much hassle for me.

So what’s the solution? The obvious solution to me was to create a shell extension. The desired behaviour is that I can right-click a folder in Windows Explorer and click “Code Review”. This will pop open a small window allowing me to input a summary and click Submit to post my review. Easy peasy.

How is this accomplished? Well, there are two pieces to this:

  • The first is to create entries in the registry that will add a new option to the context menu when the user right-clicks on a folder.
  • The second is to create a WPF application that accepts a parameter from the operating system that will define the location of the folder clicked.

Part One – Updating the Registry:

static void Main(string[] args)
{
    var menuName = @"Folder\shell\NewMenuOption";
    var command = @"Folder\shell\NewMenuOption\command";

    RegistryKey regmenu = null;
    RegistryKey regcmd = null;

    try
    {
        regmenu = Registry.ClassesRoot.CreateSubKey(menuName);
        if (regmenu != null)
            regmenu.SetValue("", "Code Review");
        regcmd = Registry.ClassesRoot.CreateSubKey(command);
        if (regcmd != null)
            regcmd.SetValue("", "C:\\Code Review\\ContextCodeReview.exe \"%1\"");
    }
    catch (Exception ex)
    {
        Console.WriteLine("Failed.");
        Console.WriteLine(ex.ToString());
        Console.ReadLine();
    }
    finally
    {
        if (regmenu != null)
            regmenu.Close();
        if (regcmd != null)
            regcmd.Close();
    }
}

So here we are creating a new listing under Folder that is going to have the name Code Review.

The next value being set is the link to the program — in this case it is called ContextCodeReview.exe along with “%1″ which will include the path to the folder clicked on when this option is invoked. This will be passed into the program as a string parameter.

We make sure to close the registry, and if there is an error it is logged to the console.

Surprised at how easy this is so far? You should also note that I put this into a program just to make it easy to be run by other team members. It is just as easy to go into the registry and add the necessary keys yourself.

The next part is also pretty easy. We just need a basic desktop application. In this case we’re using C# and WPF.

Part Two: Implementing the Windows App:

Here is the XAML for the app.

<Window x:Class="ContextCodeReview.MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        FocusManager.FocusedElement="{Binding ElementName=Summary}"
        Title="Code Review" Height="215" Width="305">
    <Grid>
        <StackPanel Orientation="Vertical">
            <StackPanel Orientation="Horizontal">
                <StackPanel Orientation="Vertical">
                    <Label Margin="5" Height="25">Summary:</Label>
                </StackPanel>
                <StackPanel Orientation="Vertical">
                    <TextBox Name="Summary" Width="145" Height="25" Margin="5"></TextBox>
                </StackPanel>
            </StackPanel>
            <StackPanel Orientation="Horizontal">
                <Button Name="SubmitBtn" Click="SubmitBtn_Click" Width="275" Margin="5">Submit</Button>
            </StackPanel>
        </StackPanel>
    </Grid>
</Window>

And the code-behind:

public partial class MainWindow : Window
{
    private string _path;

    public MainWindow()
    {
        InitializeComponent();
        _path = Environment.GetCommandLineArgs()[1];
    }

    private void SubmitBtn_Click(object sender, RoutedEventArgs e)
    {
        var process = new System.Diagnostics.Process();
        var startInfo = new System.Diagnostics.ProcessStartInfo();
        startInfo.FileName = "post-review";
        startInfo.WindowStyle = System.Diagnostics.ProcessWindowStyle.Normal;
        startInfo.Arguments = String.Format("-p --summary=\"{0}\" --target-groups=WebApps", Summary.Text);
        
        startInfo.WorkingDirectory = String.Format("{0}", _path);
        startInfo.UseShellExecute = false;
        startInfo.RedirectStandardOutput = true;
        process.StartInfo = startInfo;
        process.Start();

        string output = process.StandardOutput.ReadToEnd();

        string messageBoxText = output;
        string caption = "Result";
        MessageBoxButton mbButton = MessageBoxButton.OK;
        MessageBoxImage icon = MessageBoxImage.Information;

        MessageBox.Show(messageBoxText, caption, mbButton, icon);

        Application.Current.Shutdown();
        return;
    }
}

As you can see, it’s just a button and a text box, so I didn’t feel the need to break out MVVM or anything. It couldn’t have been easier. Once the button is click, the click handler reads the text box and inserts the contents into the summary field and publishes the review to a hard coded group.

Now, since then I’ve experienced a bit of scope creep and now there’s the ability to select your target group, publish a specific committed revision, update an existing review, and soon to be a description box. So I’ve since refactored into MVVM, but I think that’s the way of most tools people find useful.

I might end up putting this one in its current version up on github, but not right now.