JustinLee.sg

This is my last post of the year, and I hope to make it special by giving you a piece of code that just amazes me. Staying within the whole “new year” feel, this code is for validating a date.

string strMessage = string.Empty;
int index = 0;
bool IsInputValid = true;

string strDate = TextBox_Date.Text; // TextBox_Date is just a textbox to get the date from.
DateTime date;
if (strDate.Length == 10)
{
    int year = Int32.Parse(strDate.Substring(6, 4));
    if ((year > 1900) && (year < 3000))
    {
        string strMonth = strDate.Substring(3, 2);
        string strDay = strDate.Substring(0, 2);
        date = Convert.ToDateTime(strMonth + "-" + strDay + "-" + strDate.Substring(6, 4));
    }
    else
    {
        index++;
        strMessage += index + ". Date format(dd/mm/yyyy) is not correct.<br/>";
        IsInputValid = false;
    }
}
else
{
    index++;
    strMessage += index + ". Date format(dd/mm/yyyy) is not correct.<br/>";
    IsInputValid = false;
}

Remember the code you saw in Part 2 about the try-catch, using exceptions to do your data validation? Yes, that code surrounds this code. Imagine the double whammy I got when I saw this code, and the satisfaction of deleting it all.

I don’t know if you realise this, but do you have to take time to actually try to figure out what this developer is trying to do? As in understanding the code? How long do you take to figure out this code and what it is doing, and how flawed it is?

I hope you enjoy this edition of Code You Should Not Be Writing. Till next year, I have more to share!

This is a special Christmas Eve “Code You Should Not Be Writing” post that I think is special enough to flip over your chair.

This is a common mistake C# developers make, which is forgiveable. You should use string.IsNullOrEmpty() method instead, which was introduced only in .NET Framework 2.0. It is understandable that most developers haven’t gotten up to speed yet.

string duration;
// ... some code assignment for duration
if (duration == string.Empty || duration == null)
{
    // ... some error handling code
}

But this just blew my mind. Can anyone guess why this is so very wrong?

string finalTitle;
// ... some code assignment for finalTitle
if (finalTitle == string.Empty == null)
{
    // ... some error handling code
}

Yes. You should never write code like this. Heaven’s me, this is valid code! The condition is just WRONG.

Merry Christmas everyone, and I hope you like this edition of “Code You Should Not Be Writing”.