Beautifying Dates

Question

There has got to be a better way to do this. I have a method which returns either your standard "01/01/2014" Date or "January 1st, 2014" Date of the assembly file write time. Any suggestions on streamlining would be much appreciated.

// Grab the last date of revision of the assembly file write time
// You may pass any value 1-2
// 1 will return format MM/dd/YYYY
// 2 will return format MonthName Day-nth, Year
public static string RevisionDate(int id = 0)
{
    System.IO.DirectoryInfo Root = new System.IO.DirectoryInfo(HttpContext.Current.Server.MapPath("~"));
    Assembly Assy = Assembly.GetExecutingAssembly();
    string[] AssyFullName = Assy.FullName.Split(',');
    string Title = AssyFullName[0];
    string Path = null;

    if (Root.ToString().Length - 1 != ('/'))
    {
        Path = Root.ToString() + "\\bin\\" + Title + ".dll";
    }
    else
    {
        Path = Root.ToString() + "bin\\" + Title + ".dll";
    }

    var ModifiedDate = System.IO.File.GetLastWriteTime(Path);

    switch (id)
    {
        case 1:
            return Convert.ToDateTime(ModifiedDate).ToString("MM/dd/yyyy");
        case 2:
            string MonthName = Convert.ToDateTime(ModifiedDate).ToString("MMMM", CultureInfo.InvariantCulture);
            string Day = Convert.ToDateTime(ModifiedDate).ToString("dd");
            string Year = Convert.ToDateTime(ModifiedDate).ToString("yyyy");

            //eliminate leading zeros from Day
            if (Day[0] == '0')
            {
                Day = Day[Day.Length - 1].ToString();
            }

            // Beautify String
            string nth = string.Empty;
            switch (Day[Day.Length - 1]) 
            {
                case '1':
                    nth = "st";
                    break;
                case '2':
                    nth = "nd";
                    break;
                case '3':
                    nth = "rd";
                    break;
                default:
                    nth = "th";
                    break;
            }
            return MonthName + " " + Day + nth + ", " + Year;
        default:
            return "You must provide a format value 1 or 2";
    }
}

Answer 1

What will RevisionDate(5) do? I can't tell from the code. Methods should be expressed as actions: [Action][Context]; since you are returning a string I suppose GetRevisionDateById might be more appropriate (you can omit the ById part if there are no other ways of retrieving it).

---

Use using statements so you don't have to fully quality System.IO.DirectoryInfo but can instead use DirectoryInfo.

---

Local variables are written in lowerCamelCase.

---

You never use AssyFullName so you might as well just do this:

string assemblyName = Assembly.GetExecutingAssembly().FullName.Split(',');

---

This is some very obscure code:

 if (Root.ToString().Length - 1 != ('/'))

So you get a directory, take the length, substract one and compare it to the ASCII value of /? What? Unless I am reading it incorrectly, this doesn't do what you think it does. Can you give an example of a Root value so I can propose something better?

---

What exactly is id? I would connect it to a uniqueness factor of an object but here it is.. some way to describe what action to take? How do I know what 1 and 2 do?

Use an enum to describe the purpose:

enum DateStyle {
    ShortHand,
    FullBlownDateBaby
}

---

You do Convert.ToDateTime on ModifiedDate which is the result of File.GetLastWriteTime which already returns a DateTime object so that entirely unnecessary.

---

I am not great with datetime styling but I am 105% sure you are reinventing the wheel. Take a look here (standard options) and here (custom options) for all the different ways you can create your own date and time representation.

---

You're returning a string in the default statement of your switch. I would use an exception (like ArgumentException) to clearly signify that exceptional (and incorrect) input has occurred.

---

As I noted in the comments above: indenting. Braces are written on newlines (but not indented themselves) so an if statement looks like this:

if(condition) 
{
    action();
}

Answer 2

Three points to make beyond what was already said (and I strongly recommend you follow what was already said as well).

First, you ask for a day with a leading zero, then remove the leading zero. Better to just ask for the day as you want it in the first place:

string Day = Convert.ToDateTime(ModifiedDate).ToString("%d");

Now we can drop the part where we have to trim off that leading zero.

---

Second, you can DRY your code out a little by not repeating Convert.ToDateTime(ModifiedDate) multiple times. Instead, do it once and reference that:

DateTime ModifiedDate = Convert.ToDateTime(System.IO.File.GetLastWriteTime(Path));

switch (id)
{
    case 1:
        return ModifiedDate.ToString("MM/dd/yyyy");
    case 2:
        string MonthName = ModifiedDate.ToString("MMMM", CultureInfo.InvariantCulture);
        string Day = ModifiedDate.ToString("%d");
        string Year = ModifiedDate.ToString("yyyy");

---

Third, you are right at the threshold of needing to extract out at least one helper method as this is growing a little long. Case in point, your ordinal logic (adding the "st", "nd", "rd", "th") is fairly long, yet still insufficient. For the 12th day of the month, it would print "12nd". This area of code is perfect for extraction. It would be nice if C# had something to do this built in, but unfortunately it appears they do not. Fortunately, though, you aren't the first to have this problem, and someone else has already done the work for you.

Answer 3

I'll preface this with "I'm just learning C# too and I might have gotten some of the syntax wrong, but the principals are sound."

First, fix your indenting.

if (Root.ToString().Length - 1 != ('/'))
    {
Path = Root.ToString() + "\\bin\\" + Title + ".dll";
    }

Should look more like this

if (Root.ToString().Length - 1 != ('/'))
{
    Path = Root.ToString() + "\\bin\\" + Title + ".dll";
}

One problem I see is that you're using magic numbers for your argument. Set up an enum instead.

enum ArgOptions {Option1,Option2,Option3}

Then you can call your function with a meaningful argument and use that for your switch.

public static string RevisionDate(ArgOptions id = Option1) 'I'm not positive of the syntax here. I don't know c# very well.
//
// 
    switch(id)
    {
        case Option1:
            return Convert.ToDateTime(ModifiedDate).ToString("MM/dd/yyyy");
        case Option2:
            string MonthName = Convert.ToDateTime(ModifiedDate).ToString("MMMM", CultureInfo.InvariantCulture);
            string Day = Convert.ToDateTime(ModifiedDate).ToString("dd");
            string Year = Convert.ToDateTime(ModifiedDate).ToString("yyyy");

You should also throw new argument error here instead of returning a string from your function if it hits the default case.

Instead of

  default:
      return "You must provide a format value 1 or 2";

Try

throw new ArgumentException("You must provide a valid format value");