Least suprising extension methods for DateTime to NATO date-time group

Question

I have the following code to get a NATO date-time group for a DateTime. I usually need Zulu-Time or from a specific time zone. Is it clear what those methods are providing? If want to parse a NATO dtg string would those be extension methods for string or DateTime?

using System;
using System.Collections.Generic;  
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Collections.ObjectModel;
using System.Globalization;

namespace scratchpad
{
/// <summary>
/// Create NATO date-time group from DateTime
/// </summary>
static class NatoDtg
{
    /// <summary>
    /// Get NATO date-time group for UTC/Zulu timezone
    /// </summary>
    /// <param name="dt">this</param>
    /// <returns>NATO date-time group</returns>
    public static string ToZuluDtg(this DateTime dt)
    {
        DateTime utc = dt.ToUniversalTime();
        CultureInfo ci = new CultureInfo("en-GB");

        StringBuilder builder = new StringBuilder();
        builder.Append(utc.ToString("ddHHmm", ci));             // day, hours, minutes
        builder.Append("Z");                                    // time zone
        builder.Append(utc.ToString("MMMyy", ci).ToLower());    // month, year
        return builder.ToString();
    }

    /// <summary>
    /// 
    /// </summary>
    /// <param name="dt">this</param>
    /// <param name="tz">Time zone for the </param>
    /// <returns></returns>
    public static string ToDtg(this DateTime dt, TimeZoneInfo tz)
    {
        if (tz == null) throw new ArgumentNullException("TimeZoneInfo is null");
        CultureInfo ci = new CultureInfo("en-GB");
        StringBuilder builder = new StringBuilder();
        builder.Append(dt.ToString("ddHHmm", ci));             

        TimeSpan ts = tz.GetUtcOffset(dt);

        char letter = '#';
        if(ts.Hours == 0)
        {
            letter = 'Z';
        }
        else if(ts.Hours >= 1 && ts.Hours <= 9) // A-I
        {
            letter = 'A';
            letter += (char)(ts.Hours-1);
        }
        else if(ts.Hours >= 10 && ts.Hours <= 12) // K-M TRICKY: skip J (Juliet)
        {
            letter = 'A';
            letter += (char)ts.Hours;
        }
        else if(ts.Hours >= -12 && ts.Hours <= -1) // N-Y
        {
            letter = 'Z';
            letter += (char)ts.Hours;
        }
        else
        {
            throw new InvalidOperationException("Unknown UTC offset for timezone");
        }

        builder.Append(letter);
        builder.Append(dt.ToString("MMMyy", ci).ToLower());   
        return builder.ToString();
    }
}

class Program
{
    static void Main(string[] args)
    {

        DateTime now = DateTime.Now;

        Console.WriteLine("Zulu   DTG {0}", now.ToZuluDtg());
        Console.WriteLine("Berlin DTG {0}", now.ToDtg(TimeZoneInfo.GetSystemTimeZones().Where(i => i.DisplayName.Contains("Berlin")).FirstOrDefault()));
        Console.WriteLine("Local  DTG {0}", now.ToDtg(TimeZoneInfo.Local));
        Console.ReadLine();
    }
}
}

Answer 1

Firstly, I don't like the acronyms. With intellisense you can hit tab pretty early on to finish your names for you, so there's no sense adding an extra cognitive leap to work out what these functions do (even with your fairly comprehensive commenting). Call them ToDateTimeGroup and ToZuluDateTimeGroup or, even better, ToNatoDateTimeGroup and ToNatoZuluDateTimeGroup.

Speaking of commenting, your meta comments for the second method are unfinished. Additionally your commenting in-method is only commenting obvious code, which is unhelpful. Comments should explain why, not what or how, unless it's really hard to work it out (in which case, it probably needs refactoring). Additionally, comments on a new line above the code in question are generally more readable.

Your parameter names would also be better written out fully, e.g. dateTime instead of dt.

Your "Z" code is a magic string and should be a constant somewhere. It is arguable that your format strings are too.

I seem to say this a lot, but prefer to use var when the right hand side of a declaration makes the type obvious. Doing so means that should you need to change a variable's type, you only have to edit the declaration once.

I like to extend this rule to also include when the variable name makes the type obvious too, but many disagree, so your mileage may vary.

Resulting code looks like this:

/// <summary>
/// Extension methods to create NATO date-time groups from DateTime instances.
/// </summary>
static class NatoDtg
{

    private const char ZuluTimeZoneCode = `Z`;

    /// <summary>
    /// Get NATO date-time group for UTC/Zulu timezone
    /// </summary>
    /// <param name="dt">this</param>
    /// <returns>NATO date-time group</returns>
    public static string ToNatoZuluDateTimeGroup(this DateTime dateTime)
    {
        var utcTime = dateTime.ToUniversalTime();
        var culture = new CultureInfo("en-GB");

        var builder = new StringBuilder();
        builder.Append(utcTime.ToString("ddHHmm", culture));           
        builder.Append(ZuluTimeZoneCode);                                    
        builder.Append(utcTime.ToString("MMMyy", culture).ToLower());
        return builder.ToString();
    }

    /// <summary>
    /// Get NATO date-time group for a time zone
    /// </summary>
    /// <param name="dateTime">this</param>
    /// <param name="timeZone">Time zone for the date-time group </param>
    /// <returns></returns>
    public static string ToDtg(this DateTime dateTime, TimeZoneInfo timeZone)
    {
        if (timeZone == null) throw new ArgumentNullException("TimeZoneInfo is null");
        var culture = new CultureInfo("en-GB");
        var builder = new StringBuilder();
        builder.Append(dt.ToString("ddHHmm", culture));             

        TimeSpan time = timeZone.GetUtcOffset(dateTime);

        char letter = '#';
        if(ts.Hours == 0)
        {
            letter = ZuluTimeZoneCode;
        }
        else if(ts.Hours >= 1 && ts.Hours <= 9) 
        {
            //Letters A-I are based on hours
            letter = 'A';
            letter += (char)(ts.Hours-1);
        }
        else if(ts.Hours >= 10 && ts.Hours <= 12) 
        {
            //Letters K-M are based on hours, offset because we skip J
            letter = 'A';
            letter += (char)ts.Hours;
        }
        else if(ts.Hours >= -12 && ts.Hours <= -1) // N-Y
        {
            //Letters N-Y are based on hours backwards.
            letter = 'Z';
            letter += (char)ts.Hours;
        }
        else
        {
            throw new InvalidOperationException("Unknown UTC offset for timezone");
        }

        builder.Append(letter);
        builder.Append(dt.ToString("MMMyy", culture).ToLower());   
        return builder.ToString();
    }
}

class Program
{
    static void Main(string[] args)
    {

        var now = DateTime.Now;

        Console.WriteLine("Zulu   DTG {0}", now.ToZuluDtg());
        Console.WriteLine("Berlin DTG {0}", now.ToDtg(TimeZoneInfo.GetSystemTimeZones().Where(i => i.DisplayName.Contains("Berlin")).FirstOrDefault()));
        Console.WriteLine("Local  DTG {0}", now.ToDtg(TimeZoneInfo.Local));
        Console.ReadLine();
    }
}