Parsing displayed month/year for calendar

Question

I have a 6 x 7 grid for a calendar = 42 DateTimes. I'm trying to display the displayed month/year at the top of the calendar.

If there are 3 months displayed, I only want to show the name for the middle month (October 2018, for instance). If 2 months are displayed, split the name (October - November 2018). If 2 months are displayed and have different years, append with year on both sides (December 2018 - January 2019).

This is what I have, and it works, but it's in a property that may be "getted" often and I want this to be as efficient as possible. Here's the code:

public string ViewedMonthYearStr
{
    get
    {
        if (_grid != null)
        {
            string first;
            string middle = string.Empty;
            string last = string.Empty;
            HashSet<int> months = new HashSet<int> { _grid[0].Month, _grid[21].Month, _grid[41].Month };
            if (months.Count == 3) first = _grid[21].ToString("MMMM yyyy");
            else
            {
                first = _grid[0].ToString("MMMM");
                middle = " - ";
                last = _grid[41].ToString("MMMM yyyy");
            }
            if (_grid[0].Year != _grid[41].Year && months.Count != 3)
            {
                middle = $" {_grid[0].ToString("yyyy")} - ";
            }
            _viewedMonthYearStr = $"{first}{middle}{last}";
        }
        return _viewedMonthYearStr;
    }
}

Is there a more efficient way to do this? My gut tells me that if/else's aren't the most efficient way.

Answer 1

I wouldn't worry about this being inefficient - you're not doing anything crazy here, and a calendar header probably won't need to be refreshed millions of times per second anyway.

Still, I can think of some changes that will improve readability and maintainability:

• I would use a guard clause to reduce nesting, and to make it more obvious what happens when grid is null: if (_grid == null) return null;. This also gets rid of edge-cases early on, while in the current code you have to scan down to the end to see if maybe there's still an else statement following.
• Adding an empty line between the end of the else block and the next if statement will make it more obvious that they're not related.
• However, the last if statement only applies when displaying two months, so why not put it inside the above else block? That also allows you to remove the months.Count != 3 check.
• Putting first = _grid[21]... on the same line as that if statement, and without braces, makes the control flow more difficult to see at a glance. Omitting braces can also result in subtle bugs, so some people make a point of always using braces. Either way, consistency matters: if an else statement uses braces I'd expect the related if statement to also use them.
• Why assign the result to _viewMonthYearStr instead of returning it directly? That field does not seem to serve any purpose.
• 0, 21 and 41 are 'magic values': the meaning of those values isn't directly clear, and they easily break the code when the size of _grid needs to be changed. Replacing _grid[0] with _grid.First(), _grid[41] with _grid.Last() and _grid[21] with _grid[_grid.Length / 2] will make the code more robust and easier to understand. You could also assign their results to local variables, such as: var middleDate = _grid[_grid.Length / 2];. That simplifies the code somewhat, and it allows you to use _grid[0] and _grid[_grid.Length - 1] instead of First and Last without sacrificing readability, if you really need that little bit of performance gain.
• Instead of using the local variables first, middle and last, you can return results immediately. That will make the possible output formats easier to see.
• Instead of using a hash set, you can also check if the Month of the middle date is different from that of both the first and the last date. That will put a tiny bit less pressure on the GC.
• Instead of calling date.ToString(format), and then using the result in an interpolated string, you can also specify the format in the interpolated string directly: $"{date:format}".
• I'd rename ViewedMonthYearStr to Header or Title - that's what it's used for, after all.

---

With all that, this is what I would end up with:

public string Header
{
    get
    {
        if (_grid == null)
            return null;

        var firstDate = _grid.First();
        var middleDate = _grid[_grid.Length / 2];
        var lastDate = _grid.Last();

        // If more than 2 months are displayed, focus only on the middle month:
        if (middleDate.Month != firstDate.Month && middleDate.Month != lastDate.Month)
            return $"{middleDate:MMMM yyyy}";

        if (firstDate.Year != lastDate.Year)
            return $"{firstDate:MMMM yyyy} - {lastDate:MMMM yyyy}";

        return $"{firstDate:MMMM} - {lastDate:MMMM yyyy}";
    }
}