1

I have some Scala code from a game that is intended to call the onInteract(gameData: GameData, player: Player) method on each instance of a GameObject in a mutable.ArrayBuffer, gameobjects. However, sometimes these GameObjects will remove themselves from the ArrayBuffer, using the code gameobjects -= this. Since this alters the ArrayBuffer, Scala throws a NullPointerException. The code could be considered similar to the following:

for (gameobject <- gameobjects) {
    if (/* some condition */) gameobjects -= gameobject
}

and would throw an exception whenever an object is removed.

How can I remedy this? I'd imagine an ArrayBuffer, or at least the for loop, is unsuitable here.

sneelhorses
  • 175
  • 1
  • 6

2 Answers2

2

Although I can't reproduce the exception in scala 2.11.8 you can see that the result is not correct:

import scala.collection.mutable.ArrayBuffer
val a = ArrayBuffer(1, 2, 2, 3)

for (x <- a) {
   if (x == 2) a-=x
}

// There's still a 2!
a: ArrayBuffer[Int] = ArrayBuffer(1, 2, 3)

I suspect this happens since the array now has fewer elements that when you started iterating.

A better, more scala approach, is to use filter (or filterNot):

val a = ArrayBuffer(1, 2, 2, 3)
a.filter { _ != 2 } // == ArrayBuffer(1, 3)
Andy Hayden
  • 359,921
  • 101
  • 625
  • 535
  • This is similarly an issue in python: http://stackoverflow.com/q/1207406/1240268 (don't modify while iterating, you'll have a bad time!) – Andy Hayden Jun 03 '16 at 04:17
  • This changes the behavior by creating a new `ArrayBuffer` rather than modifying the existing. – vossad01 Jan 07 '17 at 04:34
1

Using -= to remove something from an ArrayBuffer is really slow--you have to search element by element to find it, then shuffle everything to the end of the buffer around. You probably ought not be doing that anyway.

If you want to iterate and delete concurrently, you should use a data structure that supports this. java.util.concurrent.ConcurrentHashMap is often a good choice if you want to look up your objects by identity and you won't have duplicates.

For example:

val chm = new java.util.concurrent.ConcurrentHashMap[String, Int]
chm put ("fish", 1); chm put ("dish", 2); chm put ("wish", 3)
val e = chm.keys
e.hasMoreElements // true
e.nextElement     // "wish"
e.hasMoreElements // true
chm remove "fish"
chm remove "dish" // empty now!!
e.nextElement     // "dish"--was going to be the next key
e.hasMoreElements // false--now it realizes chm is empty
chm get "dish"    // null, because it doesn't exist

It's a little hard to perfectly replicate this behavior with the Scala collections, so to be really safe you may want to do things longhand like above.

Rex Kerr
  • 166,841
  • 26
  • 322
  • 407
  • For completeness, do you have a recommendation when there may be duplicates (and you only wish to remove one)? – vossad01 Jan 07 '17 at 04:42
  • @vossad01 - Nothing pretty. You can always add another mutable collection as the value type, and remove the entire entry when the value is empty. (And or just keep a count, if there isn't any value associated othewise.) – Rex Kerr Jan 10 '17 at 20:35