3

I can't explain this behavior of Scala sets.

Let's start with a few definitions.

import scala.collection.mutable.Set
case class Item(name: String, content: Set[Int])
val items: Set[Item] = Set.empty

I'll add an item to my set.

items += Item("name", Set(1, 2, 3))

I'll empty my inner set.

items.filter(_.name == "name") foreach (_.content -= 1)
items
// res7: scala.collection.mutable.Set[Item] = Set(Item(name,Set(2, 3)))

So far so good.

items.filter(_.name == "name") foreach (_.content -= 2)
items.filter(_.name == "name") foreach (_.content -= 3)
items
// res12: scala.collection.mutable.Set[Item] = Set(Item(name,Set()))

Perfect! Now, what I REALLY want to do is remove the entries with an empty inner set.

items.retain(_.content.nonEmpty)
items
// res12: scala.collection.mutable.Set[Item] = Set(Item(name,Set()))

Didn't work. Maybe I did the opposite test.

items.retain(_.content.isEmpty)
items
// res14: scala.collection.mutable.Set[Item] = Set(Item(name,Set()))

Didn't work either. Maybe the filter doesn't work.

items.filter(_.content.nonEmpty)
// res15: scala.collection.mutable.Set[Item] = Set()

The filter works fine. Maybe I can't change it because it's a val.

items += Item("name", Set.empty)
items
// res17: scala.collection.mutable.Set[Item] = Set(Item(name,Set()), Item(name,Set()))

I CAN change it. And add... more of the same? Maybe they're all different.

items += Item("name", Set.empty)
items
// res19: scala.collection.mutable.Set[Item] = Set(Item(name,Set()), Item(name,Set()))

They're not all different. So can I remove any of them?

items -= Item("name", Set.empty)
items
// res21: scala.collection.mutable.Set[Item] = Set(Item(name,Set()))

I can remove ONE. Can I remove the other, the one I've been trying to remove from the start?

items -= Item("name", Set.empty)
items
// res23: scala.collection.mutable.Set[Item] = Set(Item(name,Set()))

Nope. What's happening? I'm very confused.

EDIT, SOLUTION:

Using this Stackoverflow post, Scala: Ignore case class field for equals/hascode?, I solved this by changing the way I declare the case class:

case class Item(name: String)(val content: Set[Int])

This way, the inner set is disregarded for hashcode and equals evaluations, but still accessible as a field.

Community
  • 1
  • 1
eje211
  • 2,385
  • 3
  • 28
  • 44
  • Don't ever import `mutable.Set` only ever import `mutable` then use `mutable.Set` in the scope. This is best practice because mutable should be avoided like the plague and to make it really explicit to any reader that your using mutable types. – samthebest Aug 29 '14 at 10:35
  • I tried to avoid mutable, but it just became necessary at some point. The other solution is to make the content field a `var`, and that seems much worse. Also, the value is only ever changed or queried upon getting a message in a specific actor `object`, so at least there's no real concurrency problem. – eje211 Aug 29 '14 at 17:57
  • Yes of course sometimes it is nice, particularly for low level optimizations or to avoid using recursion. The point was that you should simply make it very explicit by using `mutable.Set`, abd also u shouldnt pollute the namespace as the default is `immutable.Set` – samthebest Aug 30 '14 at 01:29
  • 1
    I did change the code today as you suggested, @samthebest. – eje211 Aug 30 '14 at 04:10

1 Answers1

6

Hashcode of Item changes when you change content. Since a set created by Set(...) is a hash set, it can't work correctly if hashes of its elements change. Note that it doesn't matter whether the Set[Item] is mutable or not; only that content is mutable.

If you put mutable objects into a hash set or use them as keys of a hash map, you must make sure that either 1) they aren't mutated while there or 2) their hashCode method is stable.

Alexey Romanov
  • 167,066
  • 35
  • 309
  • 487
  • Just checked it. If you override hashcode with i.e. name.size it works. – goral Aug 28 '14 at 05:38
  • @goral `name.hashCode` would be a bit better. – Alexey Romanov Aug 28 '14 at 05:43
  • Tried hashcode, no wonder it didn't work. Either way I just wanted to check it :) – goral Aug 28 '14 at 08:06
  • I was sure that equality and hash values were recalculated every time. Well that explains that. Also, the case class didn't have its own set until recently, so I didn't plan it that way, it just happened. On the upside, it did teach me more about Scala. Thanks. – eje211 Aug 28 '14 at 09:29
  • 2
    "I was sure that equality and hash values were recalculated every time." Yes, they are. That's precisely the problem: the element is still stored under its old hash value, but is looked up by the current hash value. – Alexey Romanov Aug 28 '14 at 10:07