How not to parse a file name

I’ve seen some pretty crazy code over the years, and this snippet that’s supposed to strip the extension from a Windows file name ranks right up there with the best of them.

    string GetFileNameWithoutExtension(string filename)
    {
        string result = filename;
        char[] charArray = filename.ToCharArray();
        Array.Reverse(charArray);
        string s1 = new string(charArray);
        string[] ext = s1.Split(new char[] {'.'}, 2);
        if (ext.Length > 1)
        {
            char[] charArray2 = ext[1].ToCharArray();
            Array.Reverse(charArray2);
            result = new string(charArray2);
        }
        return result;
    }

Perhaps the most important thing to understand about this code is that it’s totally unnecessary. There is a method, Path.GetFileNameWithoutExtension, which has been in the .NET Framework since version 1.1, that does exactly what the name implies.

But assuming the author didn’t know about that, why would he reverse, split, fold, spindle, and mutilate the string just to get the part after the last period?

When I was a kid, I was fascinated by that expression, “fold, spindle, or mutilate.” I knew what fold and mutilate were, but I just couldn’t figure out spindle. One day I was at the dry cleaners with my mom and a new employee was learning how to work the cash register. The experienced employee said, “…and then you put the ticket on the spindle,” and pushed the ticket onto the sharp pointy thing there on the counter. I still remember the feeling of the light going on in my head.

Anyway, back to the topic at hand.

This is the wonkiest code I’ve ever seen to do such a simple job. In a nutshell, it turns a filename, say “c:\folder\subdir\filename.ext” into “txe.emanelif\ridbus\redlof\:c”. Then, the call to string.Split splits it on the dot, giving two strings:

    "txe"
    "emanelif\ridbus\redlof\:c"

The code then takes the second of the two strings, reverses it, and that’s the result.

Convoluted in the extreme. Idiotically clever.

I guess the author had never heard of String.LastIndexOf, either. With that method, you can duplicate the code above in two lines:

    int dotPos = fileName.LastIndexOf('.');
    result = (dotPos == -1) ? fileName : fileName.SubString(0, dotPos);

Yep. That’s all his code does.

By the way, there’s a bug in his code and in mine. Given a file name like “c:\folder\subdir.001\filename_without_extension”, both will give a result of “c:\folder\subdir”.

To do it right, you have to take the slashes into account, too:

    string GetFileNameWithoutExtension(string filename)
    {
        int dotPos = filename.LastIndexOf('.');
        // if there's no dot, there's no extension
        if (dotPos == -1)
        {
            return filename;
        }
        int slashPos = filename.LastIndexOf('\\');
        // if the last slash is after the last dot, then there's no extension
        if (slashPos > dotPos)
        {
            return filename;
        }
        return filename.Substring(0, dotPos);
    }

I sure hope the guy who wrote that original code isn’t being paid to write computer programs. I pity his poor clients, living with those ticking time bombs in their code. Frightening.

Or perhaps I should hope for more people writing such code. I’ve made a lot of money over the years fixing crazy stuff like that.