9
\$\begingroup\$

I have this "pretty" date string generator in C# - pass it a date and it returns a string with "5 minutes ago" or "2 weeks, 3 days ago", etc.

It's a little verbose and hogs 61 lines, and I'm wondering if I'm missing out on some nice C# features or something. What (if any) is the best way to clean up this code? Are there any cool c# features I can use here?

public static string getTimeSpan(DateTime postDate)
{
    string stringy = "";
    TimeSpan diff = DateTime.Now.Subtract(postDate);
    double years = Math.Floor(diff.TotalDays / 365);
    double weeks = Math.Floor(diff.TotalDays / 7);
    double days = diff.Days;
    double hours = diff.Hours + days * 24;
    double minutes = diff.Minutes + hours * 60;
    if (minutes <= 1) {
        stringy = "Just Now";
    } else if (years >= 1) {
        if (years >= 2) {
            stringy = years.ToString() + " years ago";
        } else {
            stringy = "1 year ago";
        }
    } else if (weeks >= 1) {
        if ((days - weeks * 7) > 0) {
            if ((days - weeks * 7) > 1) {
                stringy = ", " + (days - weeks * 7).ToString() + " days";
            } else {
                stringy = ", " + (days - weeks * 7).ToString() + " day";
            }
        }
        if (weeks >= 2) {
            stringy = weeks.ToString() + " weeks" + stringy + " ago";
        } else {
            stringy = "1 week" + stringy + " ago";
        }
    } else if (days >= 1) {
        if ((hours - days * 24) > 0) {
            if ((hours - days * 24) > 1) {
                stringy = ", " + (hours - days * 24).ToString() + " hours";
            } else {
                stringy = ", " + (hours - days * 24).ToString() + " hour";
            }
        }
        if (days >= 2) {
            stringy = days.ToString() + " days" + stringy + " ago";
        } else {
            stringy = "1 day" + stringy + " ago";
        }
    } else if (hours >= 1) {
        if ((minutes - hours * 60) > 0) {
            if ((minutes - hours * 60) > 1) {
                stringy = ", " + (minutes - hours * 60).ToString() + " minutes";
            } else {
                stringy = ", " + (minutes - hours * 60).ToString() + " minute";
            }
        }
        if (hours >= 2) {
            stringy = hours.ToString() + " hours" + stringy + " ago";
        } else {
            stringy = "1 hour" + stringy + " ago";
        }
    } else if (minutes > 1) {
        stringy = minutes.ToString() + " minutes ago";
    }
    return stringy;
}
\$\endgroup\$
11
\$\begingroup\$
  1. Use PascalCase for the method name
  2. Move the calc for years and months lower to be minutely "more efficient"
  3. Use inline return to reduces nesting
  4. Use ternary operator (?:) for simple logic to reduce if/else clutter
  5. Use the format override of ToString(string format) to reduce string concats
  6. Use string.Format with a ternary to reduce duplication

The shorter version I came up with is 40 lines, but you can judge if it readable enough.

    public static string GetTimeSpan(DateTime postDate) {
        string stringy = string.Empty;
        TimeSpan diff = DateTime.Now.Subtract(postDate);
        double days = diff.Days;
        double hours = diff.Hours + days*24;
        double minutes = diff.Minutes + hours*60;
        if (minutes <= 1) {
            return "Just Now";
        }
        double years = Math.Floor(diff.TotalDays/365);
        if (years >= 1) {
            return string.Format("{0} year{1} ago", years, years >= 2 ? "s" : null);
        }
        double weeks = Math.Floor(diff.TotalDays/7);
        if (weeks >= 1) {
            double partOfWeek = days - weeks*7;
            if (partOfWeek > 0) {
                stringy = string.Format(", {0} day{1}", partOfWeek, partOfWeek > 1 ? "s" : null);
            }
            return string.Format("{0} week{1}{2} ago", weeks, weeks >= 2 ? "s" : null, stringy);
        }
        if (days >= 1) {
            double partOfDay = hours - days*24;
            if (partOfDay > 0) {
                stringy = string.Format(", {0} hour{1}", partOfDay, partOfDay > 1 ? "s" : null);
            }
            return string.Format("{0} day{1}{2} ago", days, days >= 2 ? "s" : null, stringy);
        }
        if (hours >= 1) {
            double partOfHour = minutes - hours*60;
            if (partOfHour > 0) {
                stringy = string.Format(", {0} minute{1}", partOfHour, partOfHour > 1 ? "s" : null);
            }
            return string.Format("{0} hour{1}{2} ago", hours, hours >= 2 ? "s" : null, stringy);
        }

        // Only condition left is minutes > 1
        return minutes.ToString("# minutes ago");
    }
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ wow thanks! I didn't even know about half these operators and methods and such! \$\endgroup\$ – Thomas Shields May 31 '11 at 20:46
  • \$\begingroup\$ It is important to remove last return in such cases so compiler will verify that all cases are checked and handled and there is no scenario when this method will return initial value which is string.Empty \$\endgroup\$ – Snowbear May 31 '11 at 22:05
  • \$\begingroup\$ Updated the last line to be more explicit with the final return. \$\endgroup\$ – Ed Chapel May 31 '11 at 22:30
  • 1
    \$\begingroup\$ The last line should be return string.Format("{0} minutes ago", minutes); as minutes.ToString("{0} minutes ago") will return {xxx} minutes ago. \$\endgroup\$ – mono blaine Mar 13 '19 at 14:41
  • 1
    \$\begingroup\$ Can reduce a bit of boilerplate using string interpolation instead of string.Format \$\endgroup\$ – Jesse C. Slicer Mar 14 '19 at 16:29
6
\$\begingroup\$

Some things are repeated:

days - weeks * 7
hours - days * 24
minutes - hours * 60

These can and should be made into their own variables - but what you are really after seems to be

days % 7
hours % 24
minutes % 60

You can replace:

double hours = diff.Hours + days * 24;

with

double hours = diff.TotalHours;

There is also a TotalMinutes. You can just use the Math.Floor() of these values to get an int.

I see that you are going for a single exit point for this function, but I think that readability would be improved if you got some of the simpler paths shorter:

if (minutes <= 1)         
  return "Just Now"; 

if (years >= 1) {        
  if (years >= 2) {
    return years.ToString() + " years ago";        
  } else {
    return "1 year ago";
  }

EDIT to add:

There's a repeated block of code that could be refactored to its own function:

if ((days - weeks * 7) > 0) {
   if ((days - weeks * 7) > 1) {
      stringy = ", " + (days - weeks * 7).ToString() + " days";            
   } else {
      stringy = ", " + (days - weeks * 7).ToString() + " day";
   }
}
if (weeks >= 2) {
   stringy = weeks.ToString() + " weeks" + stringy + " ago";
} else {
   stringy = "1 week" + stringy + " ago";
}

The body of the extracted function would look like:

if (smallUnitCount > 0) {
   if (smallUnitCount > 1) {
      stringy = String.Format(", {0} {1}", smallUnitCount.ToString() , smallUnitPluralName);            
   } else {
      stringy = String.Format(", {0} {1}", smallUnitCount.ToString() , smallUnitSingularName);
   }
}
if (largeUnitCount >= 2) {
   stringy = String.Format("{0} {1}{2} ago", largeUnitCount.ToString, largeUnitPluralName, stringy);
} else {
   stringy = String.Format("{0} {1}{2} ago", largeUnitCount.ToString, largeUnitSingularName, stringy);
}
| improve this answer | |
\$\endgroup\$
4
\$\begingroup\$

I would use casting double to int instead of Floor in your case. Firstly because I'm a little bit cautious about equality comparison of doubles in years >= 1. I would write it in this way:

int years = (int)(diff.TotalDays/365);
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ would doing so accomplish the same as Floor? that is, would casting .8 to an int return 0? Because that's important (i don't want .8 to round to 1 and have the function say that it's been a year when it's only really been 80% of a year) \$\endgroup\$ – Thomas Shields May 31 '11 at 21:35
  • 1
    \$\begingroup\$ Yep, with positive doubles casting to int does Floor exactly. See Explicit conversion section here. It has exactly your sample. \$\endgroup\$ – Snowbear May 31 '11 at 21:48
0
\$\begingroup\$

You can make it an extension, so you can do

string result = DateTime.Now.GetTimeSpan();

Here is how I did it a few time ago

  /// <summary>
    /// Provide extentions for the DateTime Object.
    /// </summary>
    public static class DateTimeExtensions
    {
        /// <summary>
        /// Gets the relative time for a datetime.
        /// </summary>
        /// <param name="dateTime">The datetime to get the relative time.</param>
        /// <returns>A relative time in english.</returns>
        public static string GetTimeSpan(this DateTime dateTime)
        {
            TimeSpan diff = DateTime.Now.Subtract(dateTime);

            if (diff.TotalMinutes < 1)
            {
                return string.Format("{0:D2} second{1} ago", diff.Seconds, PluralizeIfNeeded(diff.Seconds));
            }

            if (diff.TotalHours < 1)
            {
                return string.Format("{0:D2} minute{1} ago", diff.Minutes, PluralizeIfNeeded(diff.Minutes));
            }

            if (diff.TotalDays < 1)
            {
                return string.Format("{0:D2} hour{2} and {1:D2} minute{3} ago", diff.Hours, diff.Minutes, PluralizeIfNeeded(diff.Hours), PluralizeIfNeeded(diff.Minutes));
            }

            if (diff.TotalDays <= 2)
            {
                return string.Format(
                    "{0:D2} day{3}, {1:D2} hour{4} and {2:D2} minute{5} ago",
                    diff.Days,
                    diff.Hours,
                    diff.Minutes,
                    PluralizeIfNeeded(diff.Days),
                    PluralizeIfNeeded(diff.Hours),
                    PluralizeIfNeeded(diff.Minutes));
            }

            if (diff.TotalDays <= 30)
            {
                return string.Format("{0:D2} days ago", diff.TotalDays);
            }

            return string.Format("{0:g}", dateTime);
        }

        /// <summary>
        /// Gets a 's' if value is > 1.
        /// </summary>
        /// <param name="testValue">The value to test.</param>
        /// <returns>An 's' if value is > 1, otherwise an empty string.</returns>
        private static string PluralizeIfNeeded(int testValue)
        {
            return testValue > 1 ? "s" : string.Empty;
        }
    }
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.