Why does this LINQ grouping have a count of 3 instead of 2?

Question

Given the following classes:

public class WeekOfYear : IEquatable<WeekOfYear>, IComparable<WeekOfYear>
{
    private readonly DateTime dateTime;
    private readonly DayOfWeek firstDayOfWeek;

    public WeekOfYear(DateTime dateTime)
        : this(dateTime, DayOfWeek.Sunday)
    {
    }

    public WeekOfYear(DateTime dateTime, DayOfWeek firstDayOfWeek)
    {
        this.dateTime = dateTime;
        this.firstDayOfWeek = firstDayOfWeek;
    }

    public int Year
    {
        get
        {
            return dateTime.Year;
        }
    }

    public int Week
    {
        get
        {
            return CultureInfo.CurrentCulture.Calendar.GetWeekOfYear(dateTime, CalendarWeekRule.FirstDay, firstDayOfWeek);
        }
    }

    public bool Equals(WeekOfYear other)
    {
        return Year == other.Year && Week == other.Week;
    }

    public int CompareTo(WeekOfYear other)
    {
        if (Year > other.Year || Year == other.Year && Week > other.Week)
        {
            return 1;
        }
        if (Equals(other))
        {
            return 0;
        }
        return -1;
    }

    public override string ToString()
    {
        return String.Format("Week of {0}", dateTime.FirstDayOfWeek(firstDayOfWeek).ToString("MMMM dd, yyyy"));
    }
}

public class WeekOfYearComparer : IEqualityComparer<WeekOfYear>, IComparer<WeekOfYear>
{
    public bool Equals(WeekOfYear x, WeekOfYear y)
    {
        return x.Equals(y);
    }

    public int GetHashCode(WeekOfYear weekOfYear)
    {
        return weekOfYear.GetHashCode();
    }

    public int Compare(WeekOfYear x, WeekOfYear y)
    {
        return x.CompareTo(y);
    }
}

This test fails (unexpectedly):

[Test]
public void Fails()
{
    var dates = new List<DateTime>
                    {
                        new DateTime(2012, 1, 1),
                        new DateTime(2012, 2, 1),
                        new DateTime(2012, 1, 1)
                    };

    IEnumerable<IGrouping<WeekOfYear, DateTime>> groups = dates.GroupBy(date => new WeekOfYear(date), new WeekOfYearComparer());

    Assert.That(groups.Count(), Is.EqualTo(2)); // count is 3
}

And this test passes (expectedly):

[Test]
public void Works()
{
    var dates = new List<DateTime>
                    {
                        new DateTime(2012, 1, 1),
                        new DateTime(2012, 2, 1),
                        new DateTime(2012, 1, 1)
                    };

    var groups = dates.GroupBy(
        date =>
            {
                var weekOfYear = new WeekOfYear(date);
                return new { weekOfYear.Year, weekOfYear.Week };
            });

    Assert.That(groups.Count(), Is.EqualTo(2));
}

Why does the first test result in a count of 3?

score 10 · Accepted Answer · edited May 23 '17 at 11:55

The first part of an equality check is done via the hash-code; you must provide a valid hash-code implementation (for why, see Why is it important to override GetHashCode when Equals method is overridden?). Your comparer could do this, but it defers to the object:

public int GetHashCode(WeekOfYear weekOfYear)
{
    return weekOfYear.GetHashCode();
}

and the object does not provide a valid hash-code. A suitable implementation inside WeekOfYear would be something like:

public bool Equals(WeekOfYear other)
{
    return other != null && Year == other.Year && Week == other.Week;
}
public override bool Equals(object obj)
{
    return Equals(obj as WeekOfYear);
}
public override int GetHashCode()
{ // exploit number of weeks in year
    return (Year.GetHashCode()*52) + Week.GetHashCode();
}

Noting I've also provided an override for equality too.

Actually, since your object provides all the code here, there is no benefit in a custom comparer; you could remove WeekOfYearComparer completely, as the default behaviour is to look for suitable equality/comparison operations on the underlying type:

var groups = dates.GroupBy(date => new WeekOfYear(date));

Thanks, Marc. I thought it might have something to do with the hash code but was unsure. This definitely solved the problem. Cheers. — David Peden, Mar 09 '12 at 06:20

Why does this LINQ grouping have a count of 3 instead of 2?

1 Answers1