Assert uniqueness of fields in list

Question

I have made a list in C# and I want to make a test to see if all the values of the Id fields are unique.

public static List<RestaurantReview> _reviews = new List<RestaurantReview>
{
    new RestaurantReview
    {
        Id = 1,
        Name = "McDonalds",
    },
    new RestaurantReview
    {
        Id = 2,
        Name = "Burger King",
    },
}

Because I did some debugging I fount out that it is running trough the list but I do not get the proper test value. Could someone please explain what I am doing wrong here?

[TestMethod()]
public void CheckReviewIds()
{
    var FirstReview = ReviewsController._reviews.First();
    bool AllUniqueIds = ReviewsController._reviews.All(s => s.Id == FirstReview.Id);

    Assert.IsFalse(AllUniqueIds);

}

Thanks in advance.

Answer 1

Another solution that will have better performance in terms of time (specially if the data set that you have is large) is to use a HashSet like this:

bool IsAllUnique<T>(IEnumerable<T> values)
{
    HashSet<T> hash_set = new HashSet<T>();

    return values.All(x => hash_set.Add(x));
}

And then you can use it like this:

bool unique = IsAllUnique(ReviewsController._reviews.Select(x => x.Id));

This solution depends on the fact that HashSet.Add will return false if the value we are trying to add already exists.

One reason why this solution has better performance is that it does not have to go through the rest of the items in the list if it detects a duplicate item.

Answer 2

You're checking that all values are not equal to the first. This can be the case if the values are for example [1, 2, 3, 3], none are equal to the first but itself, but 3 == 3.

Instead, you can GroupBy to group them by value and then check that they are distinct. I'm assuming performance isn't a big issue here (that's the case if the list is less than 100000 items which I assume):

 ReviewsController._reviews.GroupBy(x => x.Id).Count() == ReviewsController._reviews.Count; 

---

Note that it might not be the best idea to test the internal state of a component, instead test the API it is exposing. Otherwise, the contract you define through your unit tests is limited by your implementation detail. This last paragraph is just on man's opinion though.