Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I'm looking to achieve a time format that looks like this: 2d 4h remaining; or 1h 36m remaining; or 35s remaining, etc. So that it only displays the two largest values of time (this is how Clash of Clans and some other mobile games format wait times.)

I would like to format the wait time value as simply 5h or 5h 0m depending on whether I am displaying the total wait time or currently counting down. My code is below… But is there a slightly simpler way of writing it without so many returns?

(This question was originally asked here: Is there a simpler way to format this timespan? )

public string FormatRushTime ( System.TimeSpan span, boolean removeZeros )
    if ( span.Days > 0 )
        if ( span.Hours > 0 || !removeZeros )
            return String.Format("{0:d}d {1:d}h", span.Days, span.Hours);
            return String.Format("{0:d}d", span.Days);
    if ( span.Hours > 0 )
        if ( span.Minutes > 0 || !removeZeros )
            return String.Format("{0:h}h {1:m}m", span.Hours, span.Minutes);
            return String.Format("{0:h}h", span.Hours);
    if ( span.Minutes > 0 )
        if ( span.Seconds > 0 || !removeZeros )
            return String.Format("{0:m}m {1:s}s", span.Minutes, span.Seconds);
            return String.Format("{0:m}m", span.Minutes);
    return String.Format("{0:d}s", span.Seconds);
share|improve this question
up vote 6 down vote accepted

If your goal is to reduce returns, you could use a single string.Format call and not make use of additional arguments in certain cases:

// Changed removeZeroes to includeZeroes since it read kind of like a double negative.
string format = includeZeros ? "{0:d}d {1:d}h" : "{0:d}d";

// span.Hours will not get used if there's nowhere to put it in the format string:
return string.Format(format, span.Days, span.Hours);
share|improve this answer
I like this solution the best. I didn't realize String.Format would ignore secondary variables if they were missing. This reduces the size but keeps it easily understandable. :) – Essential Jun 7 '14 at 12:36

This code is right on the tipping-point between abstracting the values to a loop, and manually unwinding the loop. To show what I mean, this is a different way of writing the function:

private static readonly string[] oneformats = {"{0:d}d", "{0:d}h", "{0:d}m", "{0:d}s"};
private static readonly string[] twoformats = {"{0:d}d {1:d}h", "{0:d}h {1:d}m", "{0:d}m {1:d}s"};

static string FormatRushTime (TimeSpan span, bool reportZero )
    int[] times = new int[]{span.Days, span.Hours, span.Minutes, span.Seconds};
    for (int i = 0; i < times.Length; i++)
        if (times[i] != 0)
            if (((i + 1) < times.Length) && (times[i+1] != 0 || reportZero))
                return String.Format(twoformats[i], times[i], times[i+1]);
            return String.Format(oneformats[i], times[i]);
    return String.Empty;

In essence, it looks in order through the array of times, and then makes a decision on whether to print just one, or the next sub-time as well.

In this way, your code overhead in terms of setting up the array, is more, but the code structure is a 'simple' loop, and does not have any 'magic' properties.

I have it running with some simple test data on ideone.

share|improve this answer

I see you reposted. My solution from

public static string FormatRushTime(System.TimeSpan span, bool removeZeroes)
    var time = new[] 
            new { Unit = "d", Value = span.Days },
            new { Unit = "h", Value = span.Hours},
            new { Unit = "m", Value = span.Minutes},
            new { Unit = "s", Value = span.Seconds}

    var filtred = time.SkipWhile(t => t.Value == 0)                             // Skip until zero values 
                      .Take(2)                                                  // How many units take
                      .Where(t => ! removeZeroes || t.Value != 0)               // Show zero values?        
                      .Select(t => String.Format("{0}{1} ", t.Value, t.Unit));

    return String.Concat(filtred);
share|improve this answer

Another possible solution.

In this solution I'm pushing all of the formatted time segments onto a stack. Then, I pop up any leading zero entries from the stack. Once I've cleared the leading zeros I then grab the top two (if they exist) from the stack and use those checking for zero on the second one. I put the two selected time segments into a length two array that is used to build the final formatted string.

This solution avoids the multi-faceted if blocks. It could be made more readable by defining your own struct to hold the time segment value and formatted string instead of the Tuple.

private string FormatRushTime( TimeSpan dt, bool includeZeros ) {

string[] dispSegments = new[] {string.Empty, "0s"};

    // Only do the work if there's an actual time span value passed in
if (dt.TotalSeconds > 0) {

            // Push the time segments from the TimeSpan onto a stack along with their
            // related string format
    Stack<Tuple<int, string>> timeSegments = new Stack<Tuple<int, string>>();

    PushTimeSegment( timeSegments, dt.Seconds, "s" );
    PushTimeSegment( timeSegments, dt.Minutes, "m" );
    PushTimeSegment( timeSegments, dt.Hours, "h" );
    PushTimeSegment( timeSegments, dt.Days, "d" );

    int takenSegments = 0;  // The number of segments already found

    // Clear out leading zeros
    while (timeSegments.Peek().Item1 == 0) {

            // Get the first two segments from the stack
    while (timeSegments.Count > 0 && takenSegments < 2) {
        Tuple<int, string> tuple = timeSegments.Pop();
        if (tuple.Item1 > 0 || includeZeros) {
            dispSegments[takenSegments] = tuple.Item2;
return string.Format( "{0} {1}", dispSegments[0], dispSegments[1] ).Trim();

private static void PushTimeSegment( Stack<Tuple<int, string>> timeSegments, int timePeriod, string timeDelimiter ) {
timeSegments.Push( new Tuple<int, string>( timePeriod, string.Format( "0:d}{1}", timePeriod, timeDelimiter ) ) );
share|improve this answer
Please give some explanation of what you did, it helps make this a review – Malachi Jun 6 '14 at 22:00
Please see my edited answer. Hopefully that explains it well enough. Let me know if you have any questions. – Mike Parkhill Jun 8 '14 at 0:17

Your Answer


By posting your answer, you agree to the privacy policy and terms of service.

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