You are currently viewing Multiplatform code review

Multiplatform code review

Hi! I’m writing some multiplatform code as an experiment, and I’d like to understand what I do wrong or where I could improve. The following is code from the common module.

class MyChat internal constructor( private val api: MyApi, override val id: String, override val language: Language, ) : WritableChat { private val listenersChannel = Channel<ChatEventListener>() private val eventChannel = Channel<ChatEvent>() private val eventsScope = CoroutineScope(Dispatchers.Default) @Volatile override var status: ChatStatus = DISCONNECTED @Volatile private lateinit var clientId: String override suspend fun connect() { check(status == DISCONNECTED) { "The chat is already connected" } val connectionInfo = api.connect(id, language) clientId = connectionInfo.clientId poll() } override suspend fun startTyping() { check(status == CONNECTED) { "The chat isn't connected" } api.startTyping(clientId) } override suspend fun stopTyping() { check(status == CONNECTED) { "The chat isn't connected" } api.stopTyping(clientId) } override suspend fun sendMessage(message: String) { check(status == CONNECTED) { "The chat isn't connected" } api.sendMessage(clientId, message) } override suspend fun disconnect() { check(status == CONNECTED) { "The chat isn't connected" } eventsScope.cancel() api.disconnect(clientId) status = DISCONNECTED } suspend fun addEventListener(listener: ChatEventListener) { listenersChannel.send(listener) } private fun poll() { eventsScope.launch { val listeners = mutableListOf<ChatEventListener>() select { listenersChannel.onReceive { listeners += it } eventChannel.onReceive { for (listener in listeners) { eventHandler(it, listener) } } } } eventsScope.launch { while (isActive) { for (event in api.queryEvents(clientId)) { eventChannel.send(event) } } } } private suspend fun eventHandler(event: ChatEvent, listener: ChatEventListener) = when (event) { ChatWaiting -> listener.chatWaiting() ChatReady -> listener.chatReady() ChatEnded -> listener.chatEnded() ... else -> error("Unexpected event '$event'") } } 

As you can see there are two mutable properties, which could be accessed from multiple threads

@Volatile override var status: ChatStatus = DISCONNECTED @Volatile private lateinit var clientId: String 

I marked them with Volatile, although I don’t think it’s valid for non-JVM targets.

Also, listeners are contained inside a coroutine

eventsScope.launch { val listeners = mutableListOf<ChatEventListener>() ... 

Is this a good approach?
What could go wrong here?

submitted by /u/lppedd
[link] [comments]