CODEWITHSUNDEEP
kotlin
Pitel
Pitel

Reputation: 5403

Removing while iterating on map

Imagine a following interface:

interface IConnectionManager {
    fun connect(connection: Int)
    fun disconnect(connection: Int)
    fun disconnectAll()
}

Simple implementation might look like this:

class ConnectionManager: IConnectionManager {
    private val connections = mutableMapOf<Int, String>()
    
    override fun connect(connection: Int) {
        connections[connection] = "Connection $connection"
    }
    
    override fun disconnect(connection: Int) {
        connections.remove(connection)?.let {
            println("Closing connection $it")
        }
    }

    override fun disconnectAll() {
        connections.forEach {
            disconnect(it.key)
        }
    }
}

Now you probably see the problem. Whenever I call disconnectAll(), I get ConcurrentModificationException.

Demo

And I know and understand why (iterators). But I can't figure a way how to implement those disconnect() and disconnectAll() methods.

I have some ideas, some of them even works, but they are ugly, and might cause bugs elsewhere:

Or is there some other, more elegant Kotlin solution?

Upvotes: 1

Views: 195

Answers (2)

Tenfour04
Tenfour04

Reputation: 93882

The quick and easy solution would be to copy keys to a list and iterate the copy:

connections.keys.toList().forEach {
    disconnect(it)
}

You could alternatively repeat your disconnect code in this method without removing them and then clear the map, but if disconnected is more complicated than the one-liner, you probably don't want to be repeating the code.

You mention the problem of another thread adding a connection, but this would be a problem no matter how you handle this. You can't simultaneously modify your collection from multiple threads. If this class needs to be thread-safe, you need to wrap each public function in a synchronized block.

Upvotes: 3

Kevin Coppock
Kevin Coppock

Reputation: 134714

Make private closeConnection(connection: Int) which wil not remove the connection from collection, and call it from both disconnect() and disconnectAll() functions. This might actualy be the best solution, but I didn't tried it yet.

This would strongly be my recommendation. Separate the actual disconnection logic from the logic that manages which connections are registered. Something like:

class ConnectionManager: IConnectionManager {
  private val connections = mutableMapOf<Int, String>()
  
  override fun connect(connection: Int) {
    connections[connection] = "Connection $connection"
  }
  
  override fun disconnect(connection: Int) {
    if (connection in connections) {
      closeConnection(connection)
      connections.remove(connection)
    }
  }

  override fun disconnectAll() {
    connections.keys.forEach(::closeConnection)
    connections.clear()
  }

  private fun closeConnection(connection: Int) {
    println("Closing connection $connection")
  }
}

Upvotes: 1

Related Questions