Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

I have read that it is bad practice to iterate over a HashSet. Should I be calling .ToList() on it first?

I have a collection of items called RegisteredItems. I do not care about the order of the items in RegisteredItems, only that they exist.

I perform two types of operations on RegisteredItems:

  • Find and return item by property.
  • Iterate over collection and have side-effect.

According to: When should I use the HashSet<T> type? Robert R. says,

"It's somewhat dangerous to iterate over a HashSet because doing so imposes an order on the items in the set. That order is not really a property of the set. You should not rely on it. If ordering of the items in a collection is important to you, that collection isn't a set."

There are some scenarios where my collection would contain 50-100 items. I realize this is not a large amount of items, but I was still hoping to reap the rewards of using a HashSet instead of List.

I have found myself looking at the following code and wondering what to do:

LayoutManager.Instance.RegisteredItems.ToList().ForEach( item => item.DoStuff() );

vs

foreach( var item in LayoutManager.Instance.RegisteredItems)
{
    item.DoStuff();
}

RegisteredItems used to return an IList<T>, but now it returns a HashSet. I felt that, if I was using HashSet for efficiency, it would be improper to cast it as a List. Yet, the above quote from Robert leaves me feeling uneasy about iterating over it, as well.

What's the right call in this scenario? Thanks

like image 953
Sean Anderson Avatar asked Nov 02 '11 16:11

Sean Anderson


2 Answers

If you don't care about order, use a HashSet<>. The quote is about using HashSet<> being dangerous when you're worried about order. If you run this code multiple times, and the items are operated on in different order, will you care? If not, then you're fine. If yes, then don't use a HashSet<>. Arbitrarily converting to a List first doesn't really solve the problem.

And I'm not certain, but I suspect that .ToList() will iterate over the HashSet<> to do that, so, now you're walking the collection twice.

Don't prematurely optimize. If you only have 100 items, just use a HashSet<> and move on. If you start caring about order, change it to a List<> then and use it as a list everwhere.

like image 200
i_am_jorf Avatar answered Nov 15 '22 10:11

i_am_jorf


If you really don't care about order and you know that you can't have duplicate in your hashset (and it's what you want), go ahead use hashset.

like image 20
Toto Avatar answered Nov 15 '22 10:11

Toto