Feedback for lambda expression to count object by DateTime.Value.Month

Question

I want to count user registrations by month, currently the code I have works, but I want some feedback or some ideas on how to do it in a better way because I think mine looks a bit messy.

Using a Unit of Work Implementation with the Repository pattern:

var repository = _unitOfWork.GetBaseRepository<Customer>();

Use Lambda expression to get the CreatedOn property from the Customers object

var customerList = await repository.GetListAsync(cancellationToken: cancellationToken);
var customers = customerList.Where(q => q.DateCreated.HasValue && q.DateCreated.Value.Date <= DateTimeOffset.UtcNow);

Initialize a new Dictionary so I can use Key-Values to store the count of users on each month.

var months = new Dictionary<string, int>
{
 {"January", 0}, {"February", 0},
 {"March", 0}, {"April", 0},
 {"May", 0}, {"June", 0},
 {"July", 0}, {"August", 0},
 {"September", 0}, {"October", 0},
 {"November", 0}, {"December", 0}
};

iterate over the customers collection and use a switch statement to match the DateTime.Month (1-12), increment the value of key by 1.

foreach (var customer in customers)
 if (customer.CreatedOn != null)
 switch (customer.CreatedOn.Value.Month)
 {
 case 1:
 months["January"] = +1;
 break;
 case 2:
 months["February"] = +1;
 break;
 case 3:
 months["March"] = +1;
 break;
 case 4:
 months["April"] = +1;
 break;
 case 5:
 months["May"] = +1;
 break;
 case 6:
 months["June"] = +1;
 break;
 case 7:
 months["July"] = +1;
 break;
 case 8:
 months["August"] = +1;
 break;
 case 9:
 months["September"] = +1;
 break;
 case 10:
 months["October"] = +1;
 break;
 case 11:
 months["November"] = +1;
 break;
 case 12:
 months["December"] = +1;
 break;
 }

iterate with foreach over the months collection using destructuring and LINQ to transform the dictionary into a JSON response

var prettifiedList = new List<string>();
foreach (var (key, value) in months) prettifiedList.Add(@$"{key}: {value}");
return await Result<List<string>>.SuccessAsync(prettifiedList);

post was originally on stackoverflow but closed due to 'opinion-based': https://stackoverflow.com/posts/70365375

• CharlesNRice: Does your UOW repo have a way to get an IQueryable? I assume this is backed by a database. Seems it would be easier to just have the database server to most of this work and not drop all the records into memory to change them.

Answer 1

You do not say whether CreatedOn is always stored in UTC or always Local or a mix. There are a few gotcha's to be aware of when working with and comparing dates.

ONE, when comparing dates, you should make sure they are on the same Kind. This clause leaves me wondering:

q.DateCreated.Value.Date <= DateTimeOffset.UtcNow

Granted the left-hand side returns a DateTime with Kind of Unspecified. That's okay, but what really matters is the Kind of the Value itself. If Value was a local time stored in the database, then the comparison to UtcNow can produce the wrong results because they are different Kinds.

TWO, when working with dates and times, most of the time you should not worry about localization of times or month names until you are displaying. Maybe you are not thinking of running your application for 2 different cultures, but it takes little effort to do so and its a good practice.

By adhering to TWO, your code would be greatly reduced by having the dictionary be keyed by month's integer value rather than the name.

var months = Enumerable.Range(1, 12).ToDictionary(key => key, value => 0);
foreach (var customer in customers)
{
 if (customer.CreatedOn.HasValue)
 {
 months[customer.CreatedOn.Value.Month]++;
 }
}

You can display that a couple of different ways. One straightforward way, I do not recommend but here it is:

You will need an enum:

public enum MonthName
{
 January = 1,
 February,
 March,
 April,
 May,
 June,
 July,
 August,
 September,
 October,
 November,
 December
}

And the code to prettify:

// The limitation here is that Month enumeration has one
// very specific, very localized spelling of a month's name.
foreach (var entry in months)
{
 Console.WriteLine($"{(MonthName)entry.Key} = {entry.Value}");
}

Note the comment. The enum provides one very specific spelling of the month's name. This might be okay with you, but if you ever want it to run for different cultures, I would recommend skipping the enumeration and let ToString do its thing:

// In this version, the MMMM will use ToString to print the
// month name it the spelling of the current culture.
foreach (var entry in months)
{
 Console.WriteLine($"{new DateTime(1, entry.Key, 1):MMMM} = {entry.Value}");
}

If an English language speaker runs the code, they get English month names. But if a French language speaker runs the code, they would get French month names.

Answer 2

The moment you find yourself copy-pasting things and only changing one particular part, you should stop yourself and reflect on how you can replace that logic with a single instance of it that uses parameters.

In this case:

• Don't do if (customer.CreatedOn != null). Add a Where clause to foreach (var customer in customers) to only retrieve the relevant records.

• CreatedOn is a nullable property, so use its .HasValue property.

• Clearly the "month translations" can be handled by a simply dictionary that maps the month's id to its name (and then use TryGetValue), or you can use the built-in functionality as documented in the answers to this StackOverlow question (e.g. CultureInfo.CurrentCulture.DateTimeFormat.GetMonthName(1) or dt.ToString("MMMM")).

Answer 3

A possible solution is to use an System.Collections.Specialized.OrderedDictionary. This allows you index the dictionary using a nifty hack with System.Linq:

OrderedDictionary months = new OrderedDictionary {
 {"January", 0}, {"February", 0},
 {"March", 0}, {"April", 0},
 {"May", 0}, {"June", 0},
 {"July", 0}, {"August", 0},
 {"September", 0}, {"October", 0},
 {"November", 0}, {"December", 0}
 };
...
foreach (var customer in customers) {
 // Guard clause here, reduces a level of indentation.
 if (customer.CreatedOn == null)
 continue;
 int index = customer.CreatedOn.Value.Month - 1;
 string MonthKey = months.Cast<DictionaryEntry>().ElementAt(index).Key.ToString();
 months[MonthKey] = +1;
}

References:

• OrderedDictionary via "max" on StackOverflow.
• Key from OrderedDictionary via "Martin R-L" on StackOverflow.