HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Beautifying Dates

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
beautifyingdatesstackoverflow

Problem

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";

Solution

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();
}

Code Snippets

string assemblyName = Assembly.GetExecutingAssembly().FullName.Split(',');
if (Root.ToString().Length - 1 != ('/'))
enum DateStyle {
    ShortHand,
    FullBlownDateBaby
}
if(condition) 
{
    action();
}

Context

StackExchange Code Review Q#52547, answer score: 5

Revisions (0)

No revisions yet.